[{"id":1764168,"web_url":"http://patchwork.ozlabs.org/comment/1764168/","msgid":"<0babc60c-8e66-e2f2-c316-c569a2ee1dfb@linux.vnet.ibm.com>","date":"2017-09-06T14:45:52","subject":"Re: [PATCH V13 4/4] powerpc/vphn: Fix numa update end-loop bug","submitter":{"id":17146,"url":"http://patchwork.ozlabs.org/api/people/17146/","name":"Nathan Fontenot","email":"nfont@linux.vnet.ibm.com"},"content":"On 09/01/2017 10:48 AM, Michael Bringmann wrote:\n> powerpc/vphn: On Power systems with shared configurations of CPUs\n> and memory, there are some issues with the association of additional\n> CPUs and memory to nodes when hot-adding resources. This patch\n> fixes an end-of-updates processing problem observed occasionally\n> in numa_update_cpu_topology().\n> \n> Signed-off-by: Michael Bringmann \n> ---\n> arch/powerpc/mm/numa.c | 7 +++++++\n> 1 file changed, 7 insertions(+)\n> \n> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c\n> index 3a5b334..fccf23f 100644\n> --- a/arch/powerpc/mm/numa.c\n> +++ b/arch/powerpc/mm/numa.c\n> @@ -1410,6 +1410,13 @@ int numa_update_cpu_topology(bool cpus_locked)\n> \t\tcpu = cpu_last_thread_sibling(cpu);\n> \t}\n> \n> +\t/*\n> +\t * Prevent processing of 'updates' from overflowing array\n> +\t * in cases where last entry filled in a 'next' pointer.\n> +\t */\n> +\tif (i)\n> +\t\tupdates[i-1].next = NULL;\n> +\n\nThis really looks like the bug is in the code above this where we\nfill in the updates array for each of the sibling cpus. The code\nthere assumes that if the current update entry is not the end that\nthere will be more updates and blindly sets the next pointer.\n\nPerhaps correcting the logic in that code to next pointers. Set the\nud pointer to NULL before the outer for_each_cpu() loop. Then in the\ninner for_each_cpu(sibling,...) loop update the ud-> next pointer as\nthe first operation.\n\n\t\tfor_each_cpu(sibling, cpu_sibling_mask(cpu)) {\n\t\t\tif (ud)\n\t\t\t\tud->next = &updates[i];\n\t\t\t...\n\t\t}\n\nObviously untested, but I think this would prevent setting the next\npointer in the last update entry that is filled out erroneously.\n \n-Nathan\n\n> \tpr_debug(\"Topology update for the following CPUs:\\n\");\n> \tif (cpumask_weight(&updated_cpus)) {\n> \t\tfor (ud = &updates[0]; ud; ud = ud->next) {\n>","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 3xnRJ72RPdz9t50\n\tfor ;\n\tThu, 7 Sep 2017 00:47:19 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xnRJ719XyzDrVw\n\tfor ;\n\tThu, 7 Sep 2017 00:47:19 +1000 (AEST)","from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com\n\t[148.163.156.1])\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 3xnRGd11JBzDrJk\n\tfor ;\n\tThu, 7 Sep 2017 00:46:00 +1000 (AEST)","from pps.filterd (m0098410.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv86EjFbh074154\n\tfor ; Wed, 6 Sep 2017 10:45:58 -0400","from e15.ny.us.ibm.com (e15.ny.us.ibm.com [129.33.205.205])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2cthcrpx9a-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Wed, 06 Sep 2017 10:45:58 -0400","from localhost\n\tby e15.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ; \n\tWed, 6 Sep 2017 10:45:57 -0400","from b01cxnp23032.gho.pok.ibm.com (9.57.198.27)\n\tby e15.ny.us.ibm.com (146.89.104.202) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tWed, 6 Sep 2017 10:45:54 -0400","from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com\n\t[9.57.199.108])\n\tby b01cxnp23032.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP\n\tid v86Ejr8j26083358; Wed, 6 Sep 2017 14:45:53 GMT","from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 985E6B205A;\n\tWed, 6 Sep 2017 10:43:16 -0400 (EDT)","from [9.41.92.186] (unknown [9.41.92.186])\n\tby b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP id 2E97DB2046;\n\tWed, 6 Sep 2017 10:43:16 -0400 (EDT)"],"Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=linux.vnet.ibm.com\n\t(client-ip=148.163.156.1; helo=mx0a-001b2d01.pphosted.com;\n\tenvelope-from=nfont@linux.vnet.ibm.com; receiver=)","Subject":"Re: [PATCH V13 4/4] powerpc/vphn: Fix numa update end-loop bug","To":"Michael Bringmann , linuxppc-dev@lists.ozlabs.org,\n\tlinux-kernel@vger.kernel.org","References":"","From":"Nathan Fontenot ","Date":"Wed, 6 Sep 2017 09:45:52 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-TM-AS-GCONF":"00","x-cbid":"17090614-0036-0000-0000-0000026500E6","X-IBM-SpamModules-Scores":"","X-IBM-SpamModules-Versions":"BY=3.00007677; HX=3.00000241; KW=3.00000007;\n\tPH=3.00000004; SC=3.00000226; SDB=6.00913113; UDB=6.00458257;\n\tIPR=6.00693375; \n\tBA=6.00005574; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009;\n\tZB=6.00000000; \n\tZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017033;\n\tXFM=3.00000015; UTC=2017-09-06 14:45:55","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17090614-0037-0000-0000-000041AD040E","Message-Id":"<0babc60c-8e66-e2f2-c316-c569a2ee1dfb@linux.vnet.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-09-06_05:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1709060207","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.23","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":"John Allen ","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n\t"}},{"id":1764411,"web_url":"http://patchwork.ozlabs.org/comment/1764411/","msgid":"<53b65d0a-65e5-2a45-dcba-bbc8e132f5bb@linux.vnet.ibm.com>","date":"2017-09-06T22:03:40","subject":"Re: [PATCH V13 4/4] powerpc/vphn: Fix numa update end-loop bug","submitter":{"id":65104,"url":"http://patchwork.ozlabs.org/api/people/65104/","name":"Michael Bringmann","email":"mwb@linux.vnet.ibm.com"},"content":"On 09/06/2017 09:45 AM, Nathan Fontenot wrote:\n> On 09/01/2017 10:48 AM, Michael Bringmann wrote:\n>> powerpc/vphn: On Power systems with shared configurations of CPUs\n>> and memory, there are some issues with the association of additional\n>> CPUs and memory to nodes when hot-adding resources. This patch\n>> fixes an end-of-updates processing problem observed occasionally\n>> in numa_update_cpu_topology().\n>>\n>> Signed-off-by: Michael Bringmann \n>> ---\n>> arch/powerpc/mm/numa.c | 7 +++++++\n>> 1 file changed, 7 insertions(+)\n>>\n>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c\n>> index 3a5b334..fccf23f 100644\n>> --- a/arch/powerpc/mm/numa.c\n>> +++ b/arch/powerpc/mm/numa.c\n>> @@ -1410,6 +1410,13 @@ int numa_update_cpu_topology(bool cpus_locked)\n>> \t\tcpu = cpu_last_thread_sibling(cpu);\n>> \t}\n>>\n>> +\t/*\n>> +\t * Prevent processing of 'updates' from overflowing array\n>> +\t * in cases where last entry filled in a 'next' pointer.\n>> +\t */\n>> +\tif (i)\n>> +\t\tupdates[i-1].next = NULL;\n>> +\n> \n> This really looks like the bug is in the code above this where we\n> fill in the updates array for each of the sibling cpus. The code\n> there assumes that if the current update entry is not the end that\n> there will be more updates and blindly sets the next pointer.\n> \n> Perhaps correcting the logic in that code to next pointers. Set the\n> ud pointer to NULL before the outer for_each_cpu() loop. Then in the\n> inner for_each_cpu(sibling,...) loop update the ud-> next pointer as\n> the first operation.\n> \n> \t\tfor_each_cpu(sibling, cpu_sibling_mask(cpu)) {\n> \t\t\tif (ud)\n> \t\t\t\tud->next = &updates[i];\n> \t\t\t...\n> \t\t}\n> \n> Obviously untested, but I think this would prevent setting the next\n> pointer in the last update entry that is filled out erroneously.\n\nThe above fragment looks to skip initialization of the 'next' pointer\nin the first element of the the 'updates'. That would abort subsequent\nevaluation of the array too soon, I believe. I would like to take another look\nto see whether the current check 'if (i < weight) ud->next = &updates[i];'\nis having problems due to i being 0-relative and weight being 1-relative.\n\n> \n> -Nathan\n\nMichael\n\n> \n>> \tpr_debug(\"Topology update for the following CPUs:\\n\");\n>> \tif (cpumask_weight(&updated_cpus)) {\n>> \t\tfor (ud = &updates[0]; ud; ud = ud->next) {\n>>\n>","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 3xnd150rcCz9rxm\n\tfor ;\n\tThu, 7 Sep 2017 08:04:57 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xnd146TF8zDrWb\n\tfor ;\n\tThu, 7 Sep 2017 08:04:56 +1000 (AEST)","from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com\n\t[148.163.158.5])\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 3xnczl3yJrzDrSS\n\tfor ;\n\tThu, 7 Sep 2017 08:03:46 +1000 (AEST)","from pps.filterd (m0098420.ppops.net [127.0.0.1])\n\tby mx0b-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv86Lxt5b108186\n\tfor ; Wed, 6 Sep 2017 18:03:44 -0400","from e14.ny.us.ibm.com (e14.ny.us.ibm.com [129.33.205.204])\n\tby mx0b-001b2d01.pphosted.com with ESMTP id 2ctr0hnu74-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Wed, 06 Sep 2017 18:03:44 -0400","from localhost\n\tby e14.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tWed, 6 Sep 2017 18:03:43 -0400","from b01cxnp23032.gho.pok.ibm.com (9.57.198.27)\n\tby e14.ny.us.ibm.com (146.89.104.201) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tWed, 6 Sep 2017 18:03:42 -0400","from b01ledav001.gho.pok.ibm.com (b01ledav001.gho.pok.ibm.com\n\t[9.57.199.106])\n\tby b01cxnp23032.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP\n\tid v86M3fkN23789754; Wed, 6 Sep 2017 22:03:41 GMT","from b01ledav001.gho.pok.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 040C12803E;\n\tWed, 6 Sep 2017 18:03:35 -0400 (EDT)","from oc1554177480.ibm.com (unknown [9.53.92.230])\n\tby b01ledav001.gho.pok.ibm.com (Postfix) with ESMTP id 94AD228046;\n\tWed, 6 Sep 2017 18:03:34 -0400 (EDT)"],"Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=linux.vnet.ibm.com\n\t(client-ip=148.163.158.5; helo=mx0a-001b2d01.pphosted.com;\n\tenvelope-from=mwb@linux.vnet.ibm.com; receiver=)","Subject":"Re: [PATCH V13 4/4] powerpc/vphn: Fix numa update end-loop bug","To":"Nathan Fontenot , linuxppc-dev@lists.ozlabs.org,\n\tlinux-kernel@vger.kernel.org","References":"\n\t<0babc60c-8e66-e2f2-c316-c569a2ee1dfb@linux.vnet.ibm.com>","From":"Michael Bringmann ","Organization":"IBM Linux Technology Center","Date":"Wed, 6 Sep 2017 17:03:40 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.0","MIME-Version":"1.0","In-Reply-To":"<0babc60c-8e66-e2f2-c316-c569a2ee1dfb@linux.vnet.ibm.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","X-TM-AS-GCONF":"00","x-cbid":"17090622-0052-0000-0000-0000025C30D6","X-IBM-SpamModules-Scores":"","X-IBM-SpamModules-Versions":"BY=3.00007679; HX=3.00000241; KW=3.00000007;\n\tPH=3.00000004; SC=3.00000226; SDB=6.00913259; UDB=6.00458344;\n\tIPR=6.00693521; \n\tBA=6.00005574; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009;\n\tZB=6.00000000; \n\tZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017040;\n\tXFM=3.00000015; UTC=2017-09-06 22:03:43","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17090622-0053-0000-0000-000051E8E3F1","Message-Id":"<53b65d0a-65e5-2a45-dcba-bbc8e132f5bb@linux.vnet.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-09-06_07:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1709060312","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.23","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":"John Allen ","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n\t"}},{"id":1764725,"web_url":"http://patchwork.ozlabs.org/comment/1764725/","msgid":"<77caa097-9d7e-93aa-bdc5-2a971687ef08@linux.vnet.ibm.com>","date":"2017-09-07T13:35:49","subject":"Re: [PATCH V13 4/4] powerpc/vphn: Fix numa update end-loop bug","submitter":{"id":17146,"url":"http://patchwork.ozlabs.org/api/people/17146/","name":"Nathan Fontenot","email":"nfont@linux.vnet.ibm.com"},"content":"On 09/06/2017 05:03 PM, Michael Bringmann wrote:\n> \n> \n> On 09/06/2017 09:45 AM, Nathan Fontenot wrote:\n>> On 09/01/2017 10:48 AM, Michael Bringmann wrote:\n>>> powerpc/vphn: On Power systems with shared configurations of CPUs\n>>> and memory, there are some issues with the association of additional\n>>> CPUs and memory to nodes when hot-adding resources. This patch\n>>> fixes an end-of-updates processing problem observed occasionally\n>>> in numa_update_cpu_topology().\n>>>\n>>> Signed-off-by: Michael Bringmann \n>>> ---\n>>> arch/powerpc/mm/numa.c | 7 +++++++\n>>> 1 file changed, 7 insertions(+)\n>>>\n>>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c\n>>> index 3a5b334..fccf23f 100644\n>>> --- a/arch/powerpc/mm/numa.c\n>>> +++ b/arch/powerpc/mm/numa.c\n>>> @@ -1410,6 +1410,13 @@ int numa_update_cpu_topology(bool cpus_locked)\n>>> \t\tcpu = cpu_last_thread_sibling(cpu);\n>>> \t}\n>>>\n>>> +\t/*\n>>> +\t * Prevent processing of 'updates' from overflowing array\n>>> +\t * in cases where last entry filled in a 'next' pointer.\n>>> +\t */\n>>> +\tif (i)\n>>> +\t\tupdates[i-1].next = NULL;\n>>> +\n>>\n>> This really looks like the bug is in the code above this where we\n>> fill in the updates array for each of the sibling cpus. The code\n>> there assumes that if the current update entry is not the end that\n>> there will be more updates and blindly sets the next pointer.\n>>\n>> Perhaps correcting the logic in that code to next pointers. Set the\n>> ud pointer to NULL before the outer for_each_cpu() loop. Then in the\n>> inner for_each_cpu(sibling,...) loop update the ud-> next pointer as\n>> the first operation.\n>>\n>> \t\tfor_each_cpu(sibling, cpu_sibling_mask(cpu)) {\n>> \t\t\tif (ud)\n>> \t\t\t\tud->next = &updates[i];\n>> \t\t\t...\n>> \t\t}\n>>\n>> Obviously untested, but I think this would prevent setting the next\n>> pointer in the last update entry that is filled out erroneously.\n> \n> The above fragment looks to skip initialization of the 'next' pointer\n> in the first element of the the 'updates'. That would abort subsequent\n> evaluation of the array too soon, I believe. I would like to take another look\n> to see whether the current check 'if (i < weight) ud->next = &updates[i];'\n> is having problems due to i being 0-relative and weight being 1-relative.\n\nAnother thing to keep in mind is that cpus can be skipped by checks earlier\nin the loop. There is not guarantee that we will add 'weight' elements to\nthe ud list.\n\n-Nathan\n \n> \n>> \n>> -Nathan\n> \n> Michael\n> \n>>\n>>> \tpr_debug(\"Topology update for the following CPUs:\\n\");\n>>> \tif (cpumask_weight(&updated_cpus)) {\n>>> \t\tfor (ud = &updates[0]; ud; ud = ud->next) {\n>>>\n>>\n>","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 3xp1hr6FZZz9s78\n\tfor ;\n\tThu, 7 Sep 2017 23:37:16 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xp1hr5Q4bzDrWN\n\tfor ;\n\tThu, 7 Sep 2017 23:37:16 +1000 (AEST)","from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com\n\t[148.163.156.1])\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 3xp1gM20CnzDrWN\n\tfor ;\n\tThu, 7 Sep 2017 23:35:59 +1000 (AEST)","from pps.filterd (m0098404.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv87DYjev135481\n\tfor ; Thu, 7 Sep 2017 09:35:57 -0400","from e13.ny.us.ibm.com (e13.ny.us.ibm.com [129.33.205.203])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2cu72y861r-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Thu, 07 Sep 2017 09:35:56 -0400","from localhost\n\tby e13.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ; \n\tThu, 7 Sep 2017 09:35:53 -0400","from b01cxnp22034.gho.pok.ibm.com (9.57.198.24)\n\tby e13.ny.us.ibm.com (146.89.104.200) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tThu, 7 Sep 2017 09:35:50 -0400","from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com\n\t[9.57.199.108])\n\tby b01cxnp22034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP\n\tid v87DZoEH41615436; Thu, 7 Sep 2017 13:35:50 GMT","from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id A13F0B2050;\n\tThu, 7 Sep 2017 09:33:12 -0400 (EDT)","from [9.41.92.186] (unknown [9.41.92.186])\n\tby b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP id 2EC6EB2046;\n\tThu, 7 Sep 2017 09:33:12 -0400 (EDT)"],"Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=linux.vnet.ibm.com\n\t(client-ip=148.163.156.1; helo=mx0a-001b2d01.pphosted.com;\n\tenvelope-from=nfont@linux.vnet.ibm.com; receiver=)","Subject":"Re: [PATCH V13 4/4] powerpc/vphn: Fix numa update end-loop bug","To":"Michael Bringmann , linuxppc-dev@lists.ozlabs.org,\n\tlinux-kernel@vger.kernel.org","References":"\n\t<0babc60c-8e66-e2f2-c316-c569a2ee1dfb@linux.vnet.ibm.com>\n\t<53b65d0a-65e5-2a45-dcba-bbc8e132f5bb@linux.vnet.ibm.com>","From":"Nathan Fontenot ","Date":"Thu, 7 Sep 2017 08:35:49 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<53b65d0a-65e5-2a45-dcba-bbc8e132f5bb@linux.vnet.ibm.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-TM-AS-GCONF":"00","x-cbid":"17090713-0008-0000-0000-0000027BCE49","X-IBM-SpamModules-Scores":"","X-IBM-SpamModules-Versions":"BY=3.00007683; HX=3.00000241; KW=3.00000007;\n\tPH=3.00000004; SC=3.00000226; SDB=6.00913569; UDB=6.00458531;\n\tIPR=6.00693827; \n\tBA=6.00005576; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009;\n\tZB=6.00000000; \n\tZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017051;\n\tXFM=3.00000015; UTC=2017-09-07 13:35:51","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17090713-0009-0000-0000-0000369ED27F","Message-Id":"<77caa097-9d7e-93aa-bdc5-2a971687ef08@linux.vnet.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-09-07_08:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1709070197","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.23","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":"John Allen ","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n\t"}},{"id":1764921,"web_url":"http://patchwork.ozlabs.org/comment/1764921/","msgid":"<756d15f9-fa17-263e-491e-7215f5ec586a@linux.vnet.ibm.com>","date":"2017-09-07T20:04:06","subject":"Re: [PATCH V13 4/4] powerpc/vphn: Fix numa update end-loop bug","submitter":{"id":65104,"url":"http://patchwork.ozlabs.org/api/people/65104/","name":"Michael Bringmann","email":"mwb@linux.vnet.ibm.com"},"content":"Simplest change IMO:\n\n\t\tfor_each_cpu(sibling, cpu_sibling_mask(cpu)) {\n\t\t\tud = &updates[i++];\n+\t\t\tud->next = &updates[i];\n ud->cpu = sibling;\n ud->new_nid = new_nid;\n ud->old_nid = numa_cpu_lookup_table[sibling];\n cpumask_set_cpu(sibling, &updated_cpus);\n- if (i < weight)\n- ud->next = &updates[i];\n }\n cpu = cpu_last_thread_sibling(cpu);\n\n\t}\n\n\tif (i)\n\t\tupdates[i-1].next = NULL;\n\nLink all of the updates together, and NULL the link pointer in the\nlast entry to be filled in. No worries about invalid comparisons.\nReduced code.\n\nMichael\n\n\nOn 09/07/2017 08:35 AM, Nathan Fontenot wrote:\n> On 09/06/2017 05:03 PM, Michael Bringmann wrote:\n>>\n>>\n>> On 09/06/2017 09:45 AM, Nathan Fontenot wrote:\n>>> On 09/01/2017 10:48 AM, Michael Bringmann wrote:\n>>>> powerpc/vphn: On Power systems with shared configurations of CPUs\n>>>> and memory, there are some issues with the association of additional\n>>>> CPUs and memory to nodes when hot-adding resources. This patch\n>>>> fixes an end-of-updates processing problem observed occasionally\n>>>> in numa_update_cpu_topology().\n>>>>\n>>>> Signed-off-by: Michael Bringmann \n>>>> ---\n>>>> arch/powerpc/mm/numa.c | 7 +++++++\n>>>> 1 file changed, 7 insertions(+)\n>>>>\n>>>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c\n>>>> index 3a5b334..fccf23f 100644\n>>>> --- a/arch/powerpc/mm/numa.c\n>>>> +++ b/arch/powerpc/mm/numa.c\n>>>> @@ -1410,6 +1410,13 @@ int numa_update_cpu_topology(bool cpus_locked)\n>>>> \t\tcpu = cpu_last_thread_sibling(cpu);\n>>>> \t}\n>>>>\n>>>> +\t/*\n>>>> +\t * Prevent processing of 'updates' from overflowing array\n>>>> +\t * in cases where last entry filled in a 'next' pointer.\n>>>> +\t */\n>>>> +\tif (i)\n>>>> +\t\tupdates[i-1].next = NULL;\n>>>> +\n>>>\n>>> This really looks like the bug is in the code above this where we\n>>> fill in the updates array for each of the sibling cpus. The code\n>>> there assumes that if the current update entry is not the end that\n>>> there will be more updates and blindly sets the next pointer.\n>>>\n>>> Perhaps correcting the logic in that code to next pointers. Set the\n>>> ud pointer to NULL before the outer for_each_cpu() loop. Then in the\n>>> inner for_each_cpu(sibling,...) loop update the ud-> next pointer as\n>>> the first operation.\n>>>\n>>> \t\tfor_each_cpu(sibling, cpu_sibling_mask(cpu)) {\n>>> \t\t\tif (ud)\n>>> \t\t\t\tud->next = &updates[i];\n>>> \t\t\t...\n>>> \t\t}\n>>>\n>>> Obviously untested, but I think this would prevent setting the next\n>>> pointer in the last update entry that is filled out erroneously.\n>>\n>> The above fragment looks to skip initialization of the 'next' pointer\n>> in the first element of the the 'updates'. That would abort subsequent\n>> evaluation of the array too soon, I believe. I would like to take another look\n>> to see whether the current check 'if (i < weight) ud->next = &updates[i];'\n>> is having problems due to i being 0-relative and weight being 1-relative.\n> \n> Another thing to keep in mind is that cpus can be skipped by checks earlier\n> in the loop. There is not guarantee that we will add 'weight' elements to\n> the ud list.\n> \n> -Nathan\n> \n>>\n>>> \n>>> -Nathan\n>>\n>> Michael\n>>\n>>>\n>>>> \tpr_debug(\"Topology update for the following CPUs:\\n\");\n>>>> \tif (cpumask_weight(&updated_cpus)) {\n>>>> \t\tfor (ud = &updates[0]; ud; ud = ud->next) {\n>>>>\n>>>\n>>\n> \n>","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 3xpBJp701jz9s83\n\tfor ;\n\tFri, 8 Sep 2017 06:05:30 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xpBJp64hnzDrb5\n\tfor ;\n\tFri, 8 Sep 2017 06:05:30 +1000 (AEST)","from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com\n\t[148.163.158.5])\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 3xpBHN2mGLzDrX7\n\tfor ;\n\tFri, 8 Sep 2017 06:04:15 +1000 (AEST)","from pps.filterd (m0098413.ppops.net [127.0.0.1])\n\tby mx0b-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv87Jxhtc109650\n\tfor ; Thu, 7 Sep 2017 16:04:12 -0400","from e32.co.us.ibm.com (e32.co.us.ibm.com [32.97.110.150])\n\tby mx0b-001b2d01.pphosted.com with ESMTP id 2cu89ppryu-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Thu, 07 Sep 2017 16:04:12 -0400","from localhost\n\tby e32.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tThu, 7 Sep 2017 14:04:11 -0600","from b03cxnp08028.gho.boulder.ibm.com (9.17.130.20)\n\tby e32.co.us.ibm.com (192.168.1.132) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tThu, 7 Sep 2017 14:04:08 -0600","from b03ledav002.gho.boulder.ibm.com\n\t(b03ledav002.gho.boulder.ibm.com [9.17.130.233])\n\tby b03cxnp08028.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with\n\tESMTP id v87K47xj30736442; Thu, 7 Sep 2017 13:04:07 -0700","from b03ledav002.gho.boulder.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id D6B4D13603A;\n\tThu, 7 Sep 2017 14:04:07 -0600 (MDT)","from oc1554177480.ibm.com (unknown [9.80.201.155])\n\tby b03ledav002.gho.boulder.ibm.com (Postfix) with ESMTP id\n\t42366136040; Thu, 7 Sep 2017 14:04:07 -0600 (MDT)"],"Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=linux.vnet.ibm.com\n\t(client-ip=148.163.158.5; helo=mx0a-001b2d01.pphosted.com;\n\tenvelope-from=mwb@linux.vnet.ibm.com; receiver=)","Subject":"Re: [PATCH V13 4/4] powerpc/vphn: Fix numa update end-loop bug","To":"linuxppc-dev@lists.ozlabs.org","References":"\n\t<0babc60c-8e66-e2f2-c316-c569a2ee1dfb@linux.vnet.ibm.com>\n\t<53b65d0a-65e5-2a45-dcba-bbc8e132f5bb@linux.vnet.ibm.com>\n\t<77caa097-9d7e-93aa-bdc5-2a971687ef08@linux.vnet.ibm.com>","From":"Michael Bringmann ","Organization":"IBM Linux Technology Center","Date":"Thu, 7 Sep 2017 15:04:06 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.0","MIME-Version":"1.0","In-Reply-To":"<77caa097-9d7e-93aa-bdc5-2a971687ef08@linux.vnet.ibm.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","X-TM-AS-GCONF":"00","x-cbid":"17090720-0004-0000-0000-000012E41AC0","X-IBM-SpamModules-Scores":"","X-IBM-SpamModules-Versions":"BY=3.00007684; HX=3.00000241; KW=3.00000007;\n\tPH=3.00000004; SC=3.00000226; SDB=6.00913698; UDB=6.00458608;\n\tIPR=6.00693957; \n\tBA=6.00005576; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009;\n\tZB=6.00000000; \n\tZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017056;\n\tXFM=3.00000015; UTC=2017-09-07 20:04:09","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17090720-0005-0000-0000-0000840041DF","Message-Id":"<756d15f9-fa17-263e-491e-7215f5ec586a@linux.vnet.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-09-07_12:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=1\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1709070297","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.23","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":"Nathan Fontenot ,\n\tJohn Allen ","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n\t"}}]