[{"id":1778924,"web_url":"http://patchwork.ozlabs.org/comment/1778924/","msgid":"<20171003125754.2kuqzkstywg7axhd@dhcp22.suse.cz>","list_archive_url":null,"date":"2017-10-03T12:57:54","subject":"Re: [PATCH v9 03/12] mm: deferred_init_memmap improvements","submitter":{"id":66979,"url":"http://patchwork.ozlabs.org/api/people/66979/","name":"Michal Hocko","email":"mhocko@kernel.org"},"content":"On Wed 20-09-17 16:17:05, Pavel Tatashin wrote:\n> This patch fixes two issues in deferred_init_memmap\n> \n> =====\n> In deferred_init_memmap() where all deferred struct pages are initialized\n> we have a check like this:\n> \n> if (page->flags) {\n> \tVM_BUG_ON(page_zone(page) != zone);\n> \tgoto free_range;\n> }\n> \n> This way we are checking if the current deferred page has already been\n> initialized. It works, because memory for struct pages has been zeroed, and\n> the only way flags are not zero if it went through __init_single_page()\n> before.  But, once we change the current behavior and won't zero the memory\n> in memblock allocator, we cannot trust anything inside \"struct page\"es\n> until they are initialized. This patch fixes this.\n> \n> The deferred_init_memmap() is re-written to loop through only free memory\n> ranges provided by memblock.\n\nPlease be explicit that this is possible only because we discard\nmemblock data later after 3010f876500f (\"mm: discard memblock data\nlater\"). Also be more explicit how the new code works.\n\nI like how the resulting code is more compact and smaller.\nfor_each_free_mem_range also looks more appropriate but I really detest\nthe DEFERRED_FREE thingy. Maybe we can handle all that in a single goto\nsection. I know this is not an art but manipulating variables from\nmacros is more error prone and much more ugly IMHO.\n\n> =====\n> This patch fixes another existing issue on systems that have holes in\n> zones i.e CONFIG_HOLES_IN_ZONE is defined.\n> \n> In for_each_mem_pfn_range() we have code like this:\n> \n> if (!pfn_valid_within(pfn)\n> \tgoto free_range;\n> \n> Note: 'page' is not set to NULL and is not incremented but 'pfn' advances.\n> Thus means if deferred struct pages are enabled on systems with these kind\n> of holes, linux would get memory corruptions. I have fixed this issue by\n> defining a new macro that performs all the necessary operations when we\n> free the current set of pages.\n\nplease do not use macros. Btw. this deserves its own fix. I suspect that\nno CONFIG_HOLES_IN_ZONE arch enables DEFERRED_STRUCT_PAGE_INIT but\npurely from the review point of view it should be its own patch.\n\n> Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com>\n> Reviewed-by: Steven Sistare <steven.sistare@oracle.com>\n> Reviewed-by: Daniel Jordan <daniel.m.jordan@oracle.com>\n> Reviewed-by: Bob Picco <bob.picco@oracle.com>\n> ---\n>  mm/page_alloc.c | 161 +++++++++++++++++++++++++++-----------------------------\n>  1 file changed, 78 insertions(+), 83 deletions(-)\n> \n> diff --git a/mm/page_alloc.c b/mm/page_alloc.c\n> index c841af88836a..d132c801d2c1 100644\n> --- a/mm/page_alloc.c\n> +++ b/mm/page_alloc.c\n> @@ -1410,14 +1410,17 @@ void clear_zone_contiguous(struct zone *zone)\n>  }\n>  \n>  #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT\n> -static void __init deferred_free_range(struct page *page,\n> -\t\t\t\t\tunsigned long pfn, int nr_pages)\n> +static void __init deferred_free_range(unsigned long pfn,\n> +\t\t\t\t       unsigned long nr_pages)\n>  {\n> -\tint i;\n> +\tstruct page *page;\n> +\tunsigned long i;\n>  \n> -\tif (!page)\n> +\tif (!nr_pages)\n>  \t\treturn;\n>  \n> +\tpage = pfn_to_page(pfn);\n> +\n>  \t/* Free a large naturally-aligned chunk if possible */\n>  \tif (nr_pages == pageblock_nr_pages &&\n>  \t    (pfn & (pageblock_nr_pages - 1)) == 0) {\n> @@ -1443,19 +1446,82 @@ static inline void __init pgdat_init_report_one_done(void)\n>  \t\tcomplete(&pgdat_init_all_done_comp);\n>  }\n>  \n> +#define DEFERRED_FREE(nr_free, free_base_pfn, page)\t\t\t\\\n> +({\t\t\t\t\t\t\t\t\t\\\n> +\tunsigned long nr = (nr_free);\t\t\t\t\t\\\n> +\t\t\t\t\t\t\t\t\t\\\n> +\tdeferred_free_range((free_base_pfn), (nr));\t\t\t\\\n> +\t(free_base_pfn) = 0;\t\t\t\t\t\t\\\n> +\t(nr_free) = 0;\t\t\t\t\t\t\t\\\n> +\tpage = NULL;\t\t\t\t\t\t\t\\\n> +\tnr;\t\t\t\t\t\t\t\t\\\n> +})\n> +\n> +static unsigned long deferred_init_range(int nid, int zid, unsigned long pfn,\n> +\t\t\t\t\t unsigned long end_pfn)\n> +{\n> +\tstruct mminit_pfnnid_cache nid_init_state = { };\n> +\tunsigned long nr_pgmask = pageblock_nr_pages - 1;\n> +\tunsigned long free_base_pfn = 0;\n> +\tunsigned long nr_pages = 0;\n> +\tunsigned long nr_free = 0;\n> +\tstruct page *page = NULL;\n> +\n> +\tfor (; pfn < end_pfn; pfn++) {\n> +\t\t/*\n> +\t\t * First we check if pfn is valid on architectures where it is\n> +\t\t * possible to have holes within pageblock_nr_pages. On systems\n> +\t\t * where it is not possible, this function is optimized out.\n> +\t\t *\n> +\t\t * Then, we check if a current large page is valid by only\n> +\t\t * checking the validity of the head pfn.\n> +\t\t *\n> +\t\t * meminit_pfn_in_nid is checked on systems where pfns can\n> +\t\t * interleave within a node: a pfn is between start and end\n> +\t\t * of a node, but does not belong to this memory node.\n> +\t\t *\n> +\t\t * Finally, we minimize pfn page lookups and scheduler checks by\n> +\t\t * performing it only once every pageblock_nr_pages.\n> +\t\t */\n> +\t\tif (!pfn_valid_within(pfn)) {\n> +\t\t\tnr_pages += DEFERRED_FREE(nr_free, free_base_pfn, page);\n> +\t\t} else if (!(pfn & nr_pgmask) && !pfn_valid(pfn)) {\n> +\t\t\tnr_pages += DEFERRED_FREE(nr_free, free_base_pfn, page);\n> +\t\t} else if (!meminit_pfn_in_nid(pfn, nid, &nid_init_state)) {\n> +\t\t\tnr_pages += DEFERRED_FREE(nr_free, free_base_pfn, page);\n> +\t\t} else if (page && (pfn & nr_pgmask)) {\n> +\t\t\tpage++;\n> +\t\t\t__init_single_page(page, pfn, zid, nid);\n> +\t\t\tnr_free++;\n> +\t\t} else {\n> +\t\t\tnr_pages += DEFERRED_FREE(nr_free, free_base_pfn, page);\n> +\t\t\tpage = pfn_to_page(pfn);\n> +\t\t\t__init_single_page(page, pfn, zid, nid);\n> +\t\t\tfree_base_pfn = pfn;\n> +\t\t\tnr_free = 1;\n> +\t\t\tcond_resched();\n> +\t\t}\n> +\t}\n> +\t/* Free the last block of pages to allocator */\n> +\tnr_pages += DEFERRED_FREE(nr_free, free_base_pfn, page);\n> +\n> +\treturn nr_pages;\n> +}\n> +\n>  /* Initialise remaining memory on a node */\n>  static int __init deferred_init_memmap(void *data)\n>  {\n>  \tpg_data_t *pgdat = data;\n>  \tint nid = pgdat->node_id;\n> -\tstruct mminit_pfnnid_cache nid_init_state = { };\n>  \tunsigned long start = jiffies;\n>  \tunsigned long nr_pages = 0;\n> -\tunsigned long walk_start, walk_end;\n> -\tint i, zid;\n> +\tunsigned long spfn, epfn;\n> +\tphys_addr_t spa, epa;\n> +\tint zid;\n>  \tstruct zone *zone;\n>  \tunsigned long first_init_pfn = pgdat->first_deferred_pfn;\n>  \tconst struct cpumask *cpumask = cpumask_of_node(pgdat->node_id);\n> +\tu64 i;\n>  \n>  \tif (first_init_pfn == ULONG_MAX) {\n>  \t\tpgdat_init_report_one_done();\n> @@ -1477,83 +1543,12 @@ static int __init deferred_init_memmap(void *data)\n>  \t\tif (first_init_pfn < zone_end_pfn(zone))\n>  \t\t\tbreak;\n>  \t}\n> +\tfirst_init_pfn = max(zone->zone_start_pfn, first_init_pfn);\n>  \n> -\tfor_each_mem_pfn_range(i, nid, &walk_start, &walk_end, NULL) {\n> -\t\tunsigned long pfn, end_pfn;\n> -\t\tstruct page *page = NULL;\n> -\t\tstruct page *free_base_page = NULL;\n> -\t\tunsigned long free_base_pfn = 0;\n> -\t\tint nr_to_free = 0;\n> -\n> -\t\tend_pfn = min(walk_end, zone_end_pfn(zone));\n> -\t\tpfn = first_init_pfn;\n> -\t\tif (pfn < walk_start)\n> -\t\t\tpfn = walk_start;\n> -\t\tif (pfn < zone->zone_start_pfn)\n> -\t\t\tpfn = zone->zone_start_pfn;\n> -\n> -\t\tfor (; pfn < end_pfn; pfn++) {\n> -\t\t\tif (!pfn_valid_within(pfn))\n> -\t\t\t\tgoto free_range;\n> -\n> -\t\t\t/*\n> -\t\t\t * Ensure pfn_valid is checked every\n> -\t\t\t * pageblock_nr_pages for memory holes\n> -\t\t\t */\n> -\t\t\tif ((pfn & (pageblock_nr_pages - 1)) == 0) {\n> -\t\t\t\tif (!pfn_valid(pfn)) {\n> -\t\t\t\t\tpage = NULL;\n> -\t\t\t\t\tgoto free_range;\n> -\t\t\t\t}\n> -\t\t\t}\n> -\n> -\t\t\tif (!meminit_pfn_in_nid(pfn, nid, &nid_init_state)) {\n> -\t\t\t\tpage = NULL;\n> -\t\t\t\tgoto free_range;\n> -\t\t\t}\n> -\n> -\t\t\t/* Minimise pfn page lookups and scheduler checks */\n> -\t\t\tif (page && (pfn & (pageblock_nr_pages - 1)) != 0) {\n> -\t\t\t\tpage++;\n> -\t\t\t} else {\n> -\t\t\t\tnr_pages += nr_to_free;\n> -\t\t\t\tdeferred_free_range(free_base_page,\n> -\t\t\t\t\t\tfree_base_pfn, nr_to_free);\n> -\t\t\t\tfree_base_page = NULL;\n> -\t\t\t\tfree_base_pfn = nr_to_free = 0;\n> -\n> -\t\t\t\tpage = pfn_to_page(pfn);\n> -\t\t\t\tcond_resched();\n> -\t\t\t}\n> -\n> -\t\t\tif (page->flags) {\n> -\t\t\t\tVM_BUG_ON(page_zone(page) != zone);\n> -\t\t\t\tgoto free_range;\n> -\t\t\t}\n> -\n> -\t\t\t__init_single_page(page, pfn, zid, nid);\n> -\t\t\tif (!free_base_page) {\n> -\t\t\t\tfree_base_page = page;\n> -\t\t\t\tfree_base_pfn = pfn;\n> -\t\t\t\tnr_to_free = 0;\n> -\t\t\t}\n> -\t\t\tnr_to_free++;\n> -\n> -\t\t\t/* Where possible, batch up pages for a single free */\n> -\t\t\tcontinue;\n> -free_range:\n> -\t\t\t/* Free the current block of pages to allocator */\n> -\t\t\tnr_pages += nr_to_free;\n> -\t\t\tdeferred_free_range(free_base_page, free_base_pfn,\n> -\t\t\t\t\t\t\t\tnr_to_free);\n> -\t\t\tfree_base_page = NULL;\n> -\t\t\tfree_base_pfn = nr_to_free = 0;\n> -\t\t}\n> -\t\t/* Free the last block of pages to allocator */\n> -\t\tnr_pages += nr_to_free;\n> -\t\tdeferred_free_range(free_base_page, free_base_pfn, nr_to_free);\n> -\n> -\t\tfirst_init_pfn = max(end_pfn, first_init_pfn);\n> +\tfor_each_free_mem_range(i, nid, MEMBLOCK_NONE, &spa, &epa, NULL) {\n> +\t\tspfn = max_t(unsigned long, first_init_pfn, PFN_UP(spa));\n> +\t\tepfn = min_t(unsigned long, zone_end_pfn(zone), PFN_DOWN(epa));\n> +\t\tnr_pages += deferred_init_range(nid, zid, spfn, epfn);\n>  \t}\n>  \n>  \t/* Sanity check that the next zone really is unpopulated */\n> -- \n> 2.14.1","headers":{"Return-Path":"<sparclinux-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y5zbZ0sJyz9sBW\n\tfor <patchwork-incoming@ozlabs.org>;\n\tTue,  3 Oct 2017 23:58:02 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753464AbdJCM6B (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tTue, 3 Oct 2017 08:58:01 -0400","from mx2.suse.de ([195.135.220.15]:45443 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1753453AbdJCM57 (ORCPT <rfc822;sparclinux@vger.kernel.org>);\n\tTue, 3 Oct 2017 08:57:59 -0400","from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id A5070AD6D;\n\tTue,  3 Oct 2017 12:57:56 +0000 (UTC)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Tue, 3 Oct 2017 14:57:54 +0200","From":"Michal Hocko <mhocko@kernel.org>","To":"Pavel Tatashin <pasha.tatashin@oracle.com>","Cc":"linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,\n\tlinux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org,\n\tlinux-s390@vger.kernel.org, linux-arm-kernel@lists.infradead.org,\n\tx86@kernel.org, kasan-dev@googlegroups.com, borntraeger@de.ibm.com,\n\theiko.carstens@de.ibm.com, davem@davemloft.net,\n\twilly@infradead.org, ard.biesheuvel@linaro.org,\n\tmark.rutland@arm.com, will.deacon@arm.com, catalin.marinas@arm.com,\n\tsam@ravnborg.org, mgorman@techsingularity.net,\n\tsteven.sistare@oracle.com, daniel.m.jordan@oracle.com,\n\tbob.picco@oracle.com","Subject":"Re: [PATCH v9 03/12] mm: deferred_init_memmap improvements","Message-ID":"<20171003125754.2kuqzkstywg7axhd@dhcp22.suse.cz>","References":"<20170920201714.19817-1-pasha.tatashin@oracle.com>\n\t<20170920201714.19817-4-pasha.tatashin@oracle.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170920201714.19817-4-pasha.tatashin@oracle.com>","User-Agent":"NeoMutt/20170609 (1.8.3)","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<sparclinux.vger.kernel.org>","X-Mailing-List":"sparclinux@vger.kernel.org"}},{"id":1779067,"web_url":"http://patchwork.ozlabs.org/comment/1779067/","msgid":"<fc4ef789-d9a8-5dab-6508-f0fe8751b462@oracle.com>","list_archive_url":null,"date":"2017-10-03T15:15:54","subject":"Re: [PATCH v9 03/12] mm: deferred_init_memmap improvements","submitter":{"id":71010,"url":"http://patchwork.ozlabs.org/api/people/71010/","name":"Pavel Tatashin","email":"pasha.tatashin@oracle.com"},"content":"Hi Michal,\n\n> \n> Please be explicit that this is possible only because we discard\n> memblock data later after 3010f876500f (\"mm: discard memblock data\n> later\"). Also be more explicit how the new code works.\n\nOK\n\n> \n> I like how the resulting code is more compact and smaller.\n\nThat was the goal :)\n\n> for_each_free_mem_range also looks more appropriate but I really detest\n> the DEFERRED_FREE thingy. Maybe we can handle all that in a single goto\n> section. I know this is not an art but manipulating variables from\n> macros is more error prone and much more ugly IMHO.\n\nSure, I can re-arrange to have a goto place. Function won't be as small, \nand if compiler is not smart enough we might end up with having more \nbranches than what my current code has.\n\n> \n> please do not use macros. Btw. this deserves its own fix. I suspect that\n> no CONFIG_HOLES_IN_ZONE arch enables DEFERRED_STRUCT_PAGE_INIT but\n> purely from the review point of view it should be its own patch.\n\nSure, I will submit this patch separately from the rest of the project. \nIn my opinion DEFERRED_STRUCT_PAGE_INIT is the way of the future, so we \nshould make sure it is working with as many configs as possible.\n\nThank you,\nPasha\n--\nTo unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<sparclinux-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y62hS4w47z9sRV\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed,  4 Oct 2017 02:17:28 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752467AbdJCPRV (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tTue, 3 Oct 2017 11:17:21 -0400","from aserp1040.oracle.com ([141.146.126.69]:32098 \"EHLO\n\taserp1040.oracle.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752129AbdJCPRS (ORCPT\n\t<rfc822; sparclinux@vger.kernel.org>); Tue, 3 Oct 2017 11:17:18 -0400","from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71])\n\tby aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with\n\tESMTP id v93FG0EA026631\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Tue, 3 Oct 2017 15:16:00 GMT","from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75])\n\tby userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id v93FFx5b019433\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Tue, 3 Oct 2017 15:15:59 GMT","from abhmp0008.oracle.com (abhmp0008.oracle.com [141.146.116.14])\n\tby userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id\n\tv93FFwks027811; Tue, 3 Oct 2017 15:15:58 GMT","from [192.168.1.10] (/98.216.35.41)\n\tby default (Oracle Beehive Gateway v4.0)\n\twith ESMTP ; Tue, 03 Oct 2017 08:15:57 -0700"],"Subject":"Re: [PATCH v9 03/12] mm: deferred_init_memmap improvements","To":"Michal Hocko <mhocko@kernel.org>","Cc":"linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,\n\tlinux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org,\n\tlinux-s390@vger.kernel.org, linux-arm-kernel@lists.infradead.org,\n\tx86@kernel.org, kasan-dev@googlegroups.com, borntraeger@de.ibm.com,\n\theiko.carstens@de.ibm.com, davem@davemloft.net,\n\twilly@infradead.org, ard.biesheuvel@linaro.org,\n\tmark.rutland@arm.com, will.deacon@arm.com, catalin.marinas@arm.com,\n\tsam@ravnborg.org, mgorman@techsingularity.net,\n\tsteven.sistare@oracle.com, daniel.m.jordan@oracle.com,\n\tbob.picco@oracle.com","References":"<20170920201714.19817-1-pasha.tatashin@oracle.com>\n\t<20170920201714.19817-4-pasha.tatashin@oracle.com>\n\t<20171003125754.2kuqzkstywg7axhd@dhcp22.suse.cz>","From":"Pasha Tatashin <pasha.tatashin@oracle.com>","Message-ID":"<fc4ef789-d9a8-5dab-6508-f0fe8751b462@oracle.com>","Date":"Tue, 3 Oct 2017 11:15:54 -0400","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20171003125754.2kuqzkstywg7axhd@dhcp22.suse.cz>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Source-IP":"userv0021.oracle.com [156.151.31.71]","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<sparclinux.vger.kernel.org>","X-Mailing-List":"sparclinux@vger.kernel.org"}},{"id":1779110,"web_url":"http://patchwork.ozlabs.org/comment/1779110/","msgid":"<d81baa49-b796-7130-4ace-0f14ed59be46@oracle.com>","list_archive_url":null,"date":"2017-10-03T16:01:08","subject":"Re: [PATCH v9 03/12] mm: deferred_init_memmap improvements","submitter":{"id":71010,"url":"http://patchwork.ozlabs.org/api/people/71010/","name":"Pavel Tatashin","email":"pasha.tatashin@oracle.com"},"content":"Hi Michal,\n\nAre you OK, if I replace DEFERRED_FREE() macro with a function like this:\n\n/*\n  * Helper for deferred_init_range, free the given range, and reset the\n  * counters\n  */\nstatic inline unsigned long __def_free(unsigned long *nr_free,\n                                        unsigned long *free_base_pfn,\n                                        struct page **page)\n{\n         unsigned long nr = *nr_free;\n\n         deferred_free_range(*free_base_pfn, nr);\n         *free_base_pfn = 0;\n         *nr_free = 0;\n         *page = NULL;\n\n         return nr;\n}\n\nSince it is inline, and we operate with non-volatile counters, compiler \nwill be smart enough to remove all the unnecessary de-references. As a \nplus, we won't be adding any new branches, and the code is still going \nto stay compact.\n\nPasha\n\nOn 10/03/2017 11:15 AM, Pasha Tatashin wrote:\n> Hi Michal,\n> \n>>\n>> Please be explicit that this is possible only because we discard\n>> memblock data later after 3010f876500f (\"mm: discard memblock data\n>> later\"). Also be more explicit how the new code works.\n> \n> OK\n> \n>>\n>> I like how the resulting code is more compact and smaller.\n> \n> That was the goal :)\n> \n>> for_each_free_mem_range also looks more appropriate but I really detest\n>> the DEFERRED_FREE thingy. Maybe we can handle all that in a single goto\n>> section. I know this is not an art but manipulating variables from\n>> macros is more error prone and much more ugly IMHO.\n> \n> Sure, I can re-arrange to have a goto place. Function won't be as small, \n> and if compiler is not smart enough we might end up with having more \n> branches than what my current code has.\n> \n>>\n>> please do not use macros. Btw. this deserves its own fix. I suspect that\n>> no CONFIG_HOLES_IN_ZONE arch enables DEFERRED_STRUCT_PAGE_INIT but\n>> purely from the review point of view it should be its own patch.\n> \n> Sure, I will submit this patch separately from the rest of the project. \n> In my opinion DEFERRED_STRUCT_PAGE_INIT is the way of the future, so we \n> should make sure it is working with as many configs as possible.\n> \n> Thank you,\n> Pasha\n--\nTo unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<sparclinux-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y63hr5xJGz9t2Q\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed,  4 Oct 2017 03:02:52 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751518AbdJCQCv (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tTue, 3 Oct 2017 12:02:51 -0400","from userp1040.oracle.com ([156.151.31.81]:38936 \"EHLO\n\tuserp1040.oracle.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750787AbdJCQCs (ORCPT\n\t<rfc822; sparclinux@vger.kernel.org>); Tue, 3 Oct 2017 12:02:48 -0400","from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234])\n\tby userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2)\n\twith ESMTP id v93G1FSG023350\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Tue, 3 Oct 2017 16:01:16 GMT","from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75])\n\tby aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id v93G1Eli030053\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Tue, 3 Oct 2017 16:01:14 GMT","from abhmp0009.oracle.com (abhmp0009.oracle.com [141.146.116.15])\n\tby userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id\n\tv93G1Bkw030011; Tue, 3 Oct 2017 16:01:11 GMT","from [192.168.1.10] (/98.216.35.41)\n\tby default (Oracle Beehive Gateway v4.0)\n\twith ESMTP ; Tue, 03 Oct 2017 09:01:10 -0700"],"Subject":"Re: [PATCH v9 03/12] mm: deferred_init_memmap improvements","From":"Pasha Tatashin <pasha.tatashin@oracle.com>","To":"Michal Hocko <mhocko@kernel.org>","Cc":"linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,\n\tlinux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org,\n\tlinux-s390@vger.kernel.org, linux-arm-kernel@lists.infradead.org,\n\tx86@kernel.org, kasan-dev@googlegroups.com, borntraeger@de.ibm.com,\n\theiko.carstens@de.ibm.com, davem@davemloft.net,\n\twilly@infradead.org, ard.biesheuvel@linaro.org,\n\tmark.rutland@arm.com, will.deacon@arm.com, catalin.marinas@arm.com,\n\tsam@ravnborg.org, mgorman@techsingularity.net,\n\tsteven.sistare@oracle.com, daniel.m.jordan@oracle.com,\n\tbob.picco@oracle.com","References":"<20170920201714.19817-1-pasha.tatashin@oracle.com>\n\t<20170920201714.19817-4-pasha.tatashin@oracle.com>\n\t<20171003125754.2kuqzkstywg7axhd@dhcp22.suse.cz>\n\t<fc4ef789-d9a8-5dab-6508-f0fe8751b462@oracle.com>","Message-ID":"<d81baa49-b796-7130-4ace-0f14ed59be46@oracle.com>","Date":"Tue, 3 Oct 2017 12:01:08 -0400","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<fc4ef789-d9a8-5dab-6508-f0fe8751b462@oracle.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Source-IP":"aserv0022.oracle.com [141.146.126.234]","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<sparclinux.vger.kernel.org>","X-Mailing-List":"sparclinux@vger.kernel.org"}},{"id":1779534,"web_url":"http://patchwork.ozlabs.org/comment/1779534/","msgid":"<20171004084816.ljyw2gdf5gmgtm7z@dhcp22.suse.cz>","list_archive_url":null,"date":"2017-10-04T08:48:16","subject":"Re: [PATCH v9 03/12] mm: deferred_init_memmap improvements","submitter":{"id":66979,"url":"http://patchwork.ozlabs.org/api/people/66979/","name":"Michal Hocko","email":"mhocko@kernel.org"},"content":"On Tue 03-10-17 12:01:08, Pasha Tatashin wrote:\n> Hi Michal,\n> \n> Are you OK, if I replace DEFERRED_FREE() macro with a function like this:\n> \n> /*\n>  * Helper for deferred_init_range, free the given range, and reset the\n>  * counters\n>  */\n> static inline unsigned long __def_free(unsigned long *nr_free,\n>                                        unsigned long *free_base_pfn,\n>                                        struct page **page)\n> {\n>         unsigned long nr = *nr_free;\n> \n>         deferred_free_range(*free_base_pfn, nr);\n>         *free_base_pfn = 0;\n>         *nr_free = 0;\n>         *page = NULL;\n> \n>         return nr;\n> }\n> \n> Since it is inline, and we operate with non-volatile counters, compiler will\n> be smart enough to remove all the unnecessary de-references. As a plus, we\n> won't be adding any new branches, and the code is still going to stay\n> compact.\n\nOK. It is a bit clunky but we are holding too much state there. I\nhaven't checked whether that can be simplified but this can be always\ndone later.","headers":{"Return-Path":"<sparclinux-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y6V110RWPz9sPt\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed,  4 Oct 2017 19:48:21 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751840AbdJDIsU (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 4 Oct 2017 04:48:20 -0400","from mx2.suse.de ([195.135.220.15]:33041 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1751736AbdJDIsT (ORCPT <rfc822;sparclinux@vger.kernel.org>);\n\tWed, 4 Oct 2017 04:48:19 -0400","from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id 92FF9AC05;\n\tWed,  4 Oct 2017 08:48:17 +0000 (UTC)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Wed, 4 Oct 2017 10:48:16 +0200","From":"Michal Hocko <mhocko@kernel.org>","To":"Pasha Tatashin <pasha.tatashin@oracle.com>","Cc":"linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,\n\tlinux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org,\n\tlinux-s390@vger.kernel.org, linux-arm-kernel@lists.infradead.org,\n\tx86@kernel.org, kasan-dev@googlegroups.com, borntraeger@de.ibm.com,\n\theiko.carstens@de.ibm.com, davem@davemloft.net,\n\twilly@infradead.org, ard.biesheuvel@linaro.org,\n\tmark.rutland@arm.com, will.deacon@arm.com, catalin.marinas@arm.com,\n\tsam@ravnborg.org, mgorman@techsingularity.net,\n\tsteven.sistare@oracle.com, daniel.m.jordan@oracle.com,\n\tbob.picco@oracle.com","Subject":"Re: [PATCH v9 03/12] mm: deferred_init_memmap improvements","Message-ID":"<20171004084816.ljyw2gdf5gmgtm7z@dhcp22.suse.cz>","References":"<20170920201714.19817-1-pasha.tatashin@oracle.com>\n\t<20170920201714.19817-4-pasha.tatashin@oracle.com>\n\t<20171003125754.2kuqzkstywg7axhd@dhcp22.suse.cz>\n\t<fc4ef789-d9a8-5dab-6508-f0fe8751b462@oracle.com>\n\t<d81baa49-b796-7130-4ace-0f14ed59be46@oracle.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<d81baa49-b796-7130-4ace-0f14ed59be46@oracle.com>","User-Agent":"NeoMutt/20170609 (1.8.3)","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<sparclinux.vger.kernel.org>","X-Mailing-List":"sparclinux@vger.kernel.org"}}]