[{"id":1778928,"web_url":"http://patchwork.ozlabs.org/comment/1778928/","msgid":"<20171003125754.2kuqzkstywg7axhd@dhcp22.suse.cz>","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 \n> Reviewed-by: Steven Sistare \n> Reviewed-by: Daniel Jordan \n> Reviewed-by: Bob Picco \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":"","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y5zd01Zyvz9sxR\n\tfor ;\n\tTue, 3 Oct 2017 23:59:16 +1100 (AEDT)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3y5zd00bN4zDqqP\n\tfor ;\n\tTue, 3 Oct 2017 23:59:16 +1100 (AEDT)","from mx1.suse.de (mx2.suse.de [195.135.220.15])\n\t(using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3y5zbY553pzDqNm\n\tfor ;\n\tTue, 3 Oct 2017 23:58:01 +1100 (AEDT)","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)"],"Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=kernel.org\n\t(client-ip=195.135.220.15; helo=mx1.suse.de;\n\tenvelope-from=mhocko@kernel.org; receiver=)","X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Tue, 3 Oct 2017 14:57:54 +0200","From":"Michal Hocko ","To":"Pavel Tatashin ","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)","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.24","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List\n\t","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"mark.rutland@arm.com, linux-s390@vger.kernel.org,\n\tard.biesheuvel@linaro.org, \n\tmgorman@techsingularity.net, sam@ravnborg.org, borntraeger@de.ibm.com,\n\twill.deacon@arm.com, x86@kernel.org, heiko.carstens@de.ibm.com,\n\tlinux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,\n\tdaniel.m.jordan@oracle.com, linux-mm@kvack.org,\n\tsteven.sistare@oracle.com, willy@infradead.org, catalin.marinas@arm.com,\n\tsparclinux@vger.kernel.org, \n\tbob.picco@oracle.com, linuxppc-dev@lists.ozlabs.org,\n\tdavem@davemloft.net, linux-arm-kernel@lists.infradead.org","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n\t"}},{"id":1779069,"web_url":"http://patchwork.ozlabs.org/comment/1779069/","msgid":"","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","headers":{"Return-Path":"","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y62jc2GYbz9sPk\n\tfor ;\n\tWed, 4 Oct 2017 02:18:28 +1100 (AEDT)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3y62jc1QB2zDqmV\n\tfor ;\n\tWed, 4 Oct 2017 02:18:28 +1100 (AEDT)","from aserp1040.oracle.com (aserp1040.oracle.com [141.146.126.69])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3y62h01YGVzDqNm\n\tfor ;\n\tWed, 4 Oct 2017 02:17:03 +1100 (AEDT)","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"],"Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=oracle.com\n\t(client-ip=141.146.126.69; helo=aserp1040.oracle.com;\n\tenvelope-from=pasha.tatashin@oracle.com; receiver=)","Subject":"Re: [PATCH v9 03/12] mm: deferred_init_memmap improvements","To":"Michal Hocko ","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 ","Message-ID":"","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]","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.24","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List\n\t","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"mark.rutland@arm.com, linux-s390@vger.kernel.org,\n\tard.biesheuvel@linaro.org, \n\tmgorman@techsingularity.net, sam@ravnborg.org, borntraeger@de.ibm.com,\n\twill.deacon@arm.com, x86@kernel.org, heiko.carstens@de.ibm.com,\n\tlinux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,\n\tdaniel.m.jordan@oracle.com, linux-mm@kvack.org,\n\tsteven.sistare@oracle.com, willy@infradead.org, catalin.marinas@arm.com,\n\tsparclinux@vger.kernel.org, \n\tbob.picco@oracle.com, linuxppc-dev@lists.ozlabs.org,\n\tdavem@davemloft.net, linux-arm-kernel@lists.infradead.org","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n\t"}},{"id":1779111,"web_url":"http://patchwork.ozlabs.org/comment/1779111/","msgid":"","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","headers":{"Return-Path":"","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y63jy3lQ0z9sRq\n\tfor ;\n\tWed, 4 Oct 2017 03:03:50 +1100 (AEDT)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3y63jy2X0zzDqrw\n\tfor ;\n\tWed, 4 Oct 2017 03:03:50 +1100 (AEDT)","from userp1040.oracle.com (userp1040.oracle.com [156.151.31.81])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3y63hb2ZZwzDqNm\n\tfor ;\n\tWed, 4 Oct 2017 03:02:38 +1100 (AEDT)","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"],"Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=oracle.com\n\t(client-ip=156.151.31.81; helo=userp1040.oracle.com;\n\tenvelope-from=pasha.tatashin@oracle.com; receiver=)","Subject":"Re: [PATCH v9 03/12] mm: deferred_init_memmap improvements","From":"Pasha Tatashin ","To":"Michal Hocko ","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","Message-ID":"","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":"","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]","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.24","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List\n\t","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"mark.rutland@arm.com, linux-s390@vger.kernel.org,\n\tard.biesheuvel@linaro.org, \n\tmgorman@techsingularity.net, sam@ravnborg.org, borntraeger@de.ibm.com,\n\twill.deacon@arm.com, x86@kernel.org, heiko.carstens@de.ibm.com,\n\tlinux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,\n\tdaniel.m.jordan@oracle.com, linux-mm@kvack.org,\n\tsteven.sistare@oracle.com, willy@infradead.org, catalin.marinas@arm.com,\n\tsparclinux@vger.kernel.org, \n\tbob.picco@oracle.com, linuxppc-dev@lists.ozlabs.org,\n\tdavem@davemloft.net, linux-arm-kernel@lists.infradead.org","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n\t"}},{"id":1779536,"web_url":"http://patchwork.ozlabs.org/comment/1779536/","msgid":"<20171004084816.ljyw2gdf5gmgtm7z@dhcp22.suse.cz>","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":"","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y6V2S3Wl8z9sRV\n\tfor ;\n\tWed, 4 Oct 2017 19:49:36 +1100 (AEDT)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3y6V2S2gQlzDrCl\n\tfor ;\n\tWed, 4 Oct 2017 19:49:36 +1100 (AEDT)","from mx1.suse.de (mx2.suse.de [195.135.220.15])\n\t(using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3y6V1052D2zDr4d\n\tfor ;\n\tWed, 4 Oct 2017 19:48:20 +1100 (AEDT)","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)"],"Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=kernel.org\n\t(client-ip=195.135.220.15; helo=mx1.suse.de;\n\tenvelope-from=mhocko@kernel.org; receiver=)","X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Wed, 4 Oct 2017 10:48:16 +0200","From":"Michal Hocko ","To":"Pasha Tatashin ","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\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"NeoMutt/20170609 (1.8.3)","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.24","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List\n\t","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"mark.rutland@arm.com, linux-s390@vger.kernel.org,\n\tard.biesheuvel@linaro.org, \n\tmgorman@techsingularity.net, sam@ravnborg.org, borntraeger@de.ibm.com,\n\twill.deacon@arm.com, x86@kernel.org, heiko.carstens@de.ibm.com,\n\tlinux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,\n\tdaniel.m.jordan@oracle.com, linux-mm@kvack.org,\n\tsteven.sistare@oracle.com, willy@infradead.org, catalin.marinas@arm.com,\n\tsparclinux@vger.kernel.org, \n\tbob.picco@oracle.com, linuxppc-dev@lists.ozlabs.org,\n\tdavem@davemloft.net, linux-arm-kernel@lists.infradead.org","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n\t"}}]