[{"id":1778896,"web_url":"http://patchwork.ozlabs.org/comment/1778896/","msgid":"<20171003122658.cv64pxnuavopjid6@dhcp22.suse.cz>","date":"2017-10-03T12:26:58","subject":"Re: [PATCH v9 01/12] x86/mm: setting fields in deferred pages","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:03, Pavel Tatashin wrote:\n> Without deferred struct page feature (CONFIG_DEFERRED_STRUCT_PAGE_INIT),\n> flags and other fields in \"struct page\"es are never changed prior to first\n> initializing struct pages by going through __init_single_page().\n> \n> With deferred struct page feature enabled, however, we set fields in\n> register_page_bootmem_info that are subsequently clobbered right after in\n> free_all_bootmem:\n> \n>         mem_init() {\n>                 register_page_bootmem_info();\n>                 free_all_bootmem();\n>                 ...\n>         }\n> \n> When register_page_bootmem_info() is called only non-deferred struct pages\n> are initialized. But, this function goes through some reserved pages which\n> might be part of the deferred, and thus are not yet initialized.\n> \n>   mem_init\n>    register_page_bootmem_info\n>     register_page_bootmem_info_node\n>      get_page_bootmem\n>       .. setting fields here ..\n>       such as: page->freelist = (void *)type;\n> \n>   free_all_bootmem()\n>    free_low_memory_core_early()\n>     for_each_reserved_mem_region()\n>      reserve_bootmem_region()\n>       init_reserved_page() <- Only if this is deferred reserved page\n>        __init_single_pfn()\n>         __init_single_page()\n>             memset(0) <-- Loose the set fields here\n> \n> We end-up with issue where, currently we do not observe problem as memory\n> is explicitly zeroed. But, if flag asserts are changed we can start hitting\n> issues.\n> \n> Also, because in this patch series we will stop zeroing struct page memory\n> during allocation, we must make sure that struct pages are properly\n> initialized prior to using them.\n> \n> The deferred-reserved pages are initialized in free_all_bootmem().\n> Therefore, the fix is to switch the above calls.\n\nThanks for extending the changelog. This is more informative now.\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\nI hope I haven't missed anything but it looks good to me.\n\nAcked-by: Michal Hocko <mhocko@suse.com>\n\none nit below\n> ---\n>  arch/x86/mm/init_64.c | 9 +++++++--\n>  1 file changed, 7 insertions(+), 2 deletions(-)\n> \n> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c\n> index 5ea1c3c2636e..30fe22558720 100644\n> --- a/arch/x86/mm/init_64.c\n> +++ b/arch/x86/mm/init_64.c\n> @@ -1182,12 +1182,17 @@ void __init mem_init(void)\n>  \n>  \t/* clear_bss() already clear the empty_zero_page */\n>  \n> -\tregister_page_bootmem_info();\n> -\n>  \t/* this will put all memory onto the freelists */\n>  \tfree_all_bootmem();\n>  \tafter_bootmem = 1;\n>  \n> +\t/* Must be done after boot memory is put on freelist, because here we\n\nstandard code style is to do\n\t/*\n\t * text starts here\n\n> +\t * might set fields in deferred struct pages that have not yet been\n> +\t * initialized, and free_all_bootmem() initializes all the reserved\n> +\t * deferred pages for us.\n> +\t */\n> +\tregister_page_bootmem_info();\n> +\n>  \t/* Register memory areas for /proc/kcore */\n>  \tkclist_add(&kcore_vsyscall, (void *)VSYSCALL_ADDR,\n>  \t\t\t PAGE_SIZE, KCORE_OTHER);\n> -- \n> 2.14.1","headers":{"Return-Path":"<linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>","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 3y5yxh1JFlz9ryk\n\tfor <patchwork-incoming@ozlabs.org>;\n\tTue,  3 Oct 2017 23:28:40 +1100 (AEDT)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3y5yxh00ZwzDqw7\n\tfor <patchwork-incoming@ozlabs.org>;\n\tTue,  3 Oct 2017 23:28:40 +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 3y5yvt607RzDqnd\n\tfor <linuxppc-dev@lists.ozlabs.org>;\n\tTue,  3 Oct 2017 23:27:06 +1100 (AEDT)","from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id 6F558AC68;\n\tTue,  3 Oct 2017 12:27:00 +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=<UNKNOWN>)","X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Tue, 3 Oct 2017 14:26:58 +0200","From":"Michal Hocko <mhocko@kernel.org>","To":"Pavel Tatashin <pasha.tatashin@oracle.com>","Subject":"Re: [PATCH v9 01/12] x86/mm: setting fields in deferred pages","Message-ID":"<20171003122658.cv64pxnuavopjid6@dhcp22.suse.cz>","References":"<20170920201714.19817-1-pasha.tatashin@oracle.com>\n\t<20170920201714.19817-2-pasha.tatashin@oracle.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170920201714.19817-2-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<linuxppc-dev.lists.ozlabs.org>","List-Unsubscribe":"<https://lists.ozlabs.org/options/linuxppc-dev>,\n\t<mailto:linuxppc-dev-request@lists.ozlabs.org?subject=unsubscribe>","List-Archive":"<http://lists.ozlabs.org/pipermail/linuxppc-dev/>","List-Post":"<mailto:linuxppc-dev@lists.ozlabs.org>","List-Help":"<mailto:linuxppc-dev-request@lists.ozlabs.org?subject=help>","List-Subscribe":"<https://lists.ozlabs.org/listinfo/linuxppc-dev>,\n\t<mailto:linuxppc-dev-request@lists.ozlabs.org?subject=subscribe>","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<linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>"}},{"id":1779060,"web_url":"http://patchwork.ozlabs.org/comment/1779060/","msgid":"<00978c7c-8d05-fbc3-eaba-9455b66ff02e@oracle.com>","date":"2017-10-03T15:07:54","subject":"Re: [PATCH v9 01/12] x86/mm: setting fields in deferred pages","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> I hope I haven't missed anything but it looks good to me.\n> \n> Acked-by: Michal Hocko <mhocko@suse.com>\n\nThank you for your review.\n\n> \n> one nit below\n>> ---\n>>   arch/x86/mm/init_64.c | 9 +++++++--\n>>   1 file changed, 7 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c\n>> index 5ea1c3c2636e..30fe22558720 100644\n>> --- a/arch/x86/mm/init_64.c\n>> +++ b/arch/x86/mm/init_64.c\n>> @@ -1182,12 +1182,17 @@ void __init mem_init(void)\n>>   \n>>   \t/* clear_bss() already clear the empty_zero_page */\n>>   \n>> -\tregister_page_bootmem_info();\n>> -\n>>   \t/* this will put all memory onto the freelists */\n>>   \tfree_all_bootmem();\n>>   \tafter_bootmem = 1;\n>>   \n>> +\t/* Must be done after boot memory is put on freelist, because here we\n> \n> standard code style is to do\n> \t/*\n> \t * text starts here\n\nOK, will change for both patch 1 and 2.\n\nPasha","headers":{"Return-Path":"<linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>","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 3y62XX3v4Gz9sPk\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed,  4 Oct 2017 02:10: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 3y62XX1kWMzDqlF\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed,  4 Oct 2017 02:10:36 +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 3y62Vr2FGszDqks\n\tfor <linuxppc-dev@lists.ozlabs.org>;\n\tWed,  4 Oct 2017 02:09:08 +1100 (AEDT)","from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74])\n\tby aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with\n\tESMTP id v93F7wEU011444\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Tue, 3 Oct 2017 15:07:59 GMT","from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75])\n\tby userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id v93F7wGL025883\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Tue, 3 Oct 2017 15:07:58 GMT","from abhmp0002.oracle.com (abhmp0002.oracle.com [141.146.116.8])\n\tby userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id v93F7vLL022241; \n\tTue, 3 Oct 2017 15:07:57 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:07: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=<UNKNOWN>)","Subject":"Re: [PATCH v9 01/12] x86/mm: setting fields in deferred pages","To":"Michal Hocko <mhocko@kernel.org>","References":"<20170920201714.19817-1-pasha.tatashin@oracle.com>\n\t<20170920201714.19817-2-pasha.tatashin@oracle.com>\n\t<20171003122658.cv64pxnuavopjid6@dhcp22.suse.cz>","From":"Pasha Tatashin <pasha.tatashin@oracle.com>","Message-ID":"<00978c7c-8d05-fbc3-eaba-9455b66ff02e@oracle.com>","Date":"Tue, 3 Oct 2017 11:07: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":"<20171003122658.cv64pxnuavopjid6@dhcp22.suse.cz>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Source-IP":"userv0022.oracle.com [156.151.31.74]","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<linuxppc-dev.lists.ozlabs.org>","List-Unsubscribe":"<https://lists.ozlabs.org/options/linuxppc-dev>,\n\t<mailto:linuxppc-dev-request@lists.ozlabs.org?subject=unsubscribe>","List-Archive":"<http://lists.ozlabs.org/pipermail/linuxppc-dev/>","List-Post":"<mailto:linuxppc-dev@lists.ozlabs.org>","List-Help":"<mailto:linuxppc-dev-request@lists.ozlabs.org?subject=help>","List-Subscribe":"<https://lists.ozlabs.org/listinfo/linuxppc-dev>,\n\t<mailto:linuxppc-dev-request@lists.ozlabs.org?subject=subscribe>","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<linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org>"}}]