Received-SPF: pass (google.com: domain of linux-kernel+bounces-30834-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <c7ad2b9f-0ff2-4f08-a666-015284a289dc@amperemail.onmicrosoft.com>
Date: Fri, 19 Jan 2024 15:02:29 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] NUMA: Early use of cpu_to_node() returns 0 instead of the
 correct node id
From: Shijie Huang <shijie@amperemail.onmicrosoft.com>
To: Yury Norov <yury.norov@gmail.com>,
 Huang Shijie <shijie@os.amperecomputing.com>
Cc: gregkh@linuxfoundation.org, patches@amperecomputing.com,
 rafael@kernel.org, paul.walmsley@sifive.com, palmer@dabbelt.com,
 aou@eecs.berkeley.edu, kuba@kernel.org, vschneid@redhat.com,
 mingo@kernel.org, akpm@linux-foundation.org, vbabka@suse.cz,
 rppt@kernel.org, tglx@linutronix.de, jpoimboe@kernel.org,
 ndesaulniers@google.com, mikelley@microsoft.com, mhiramat@kernel.org,
 arnd@arndb.de, linux-kernel@vger.kernel.org,
 linux-riscv@lists.infradead.org, linux-arm-kernel@lists.infradead.org,
 catalin.marinas@arm.com, will@kernel.org, mark.rutland@arm.com,
 mpe@ellerman.id.au, linuxppc-dev@lists.ozlabs.org, chenhuacai@kernel.org,
 jiaxun.yang@flygoat.com, linux-mips@vger.kernel.org,
 cl@os.amperecomputing.com
References: <20240119033227.14113-1-shijie@os.amperecomputing.com>
 <Zan9sb0vtSvVvQeA@yury-ThinkPad>
 <1cd078fd-c345-4d85-a92f-04c806c20efa@amperemail.onmicrosoft.com>
In-Reply-To: <1cd078fd-c345-4d85-a92f-04c806c20efa@amperemail.onmicrosoft.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?MzJybVNUZjc0SFNDdDkyWUNKVHZJVXlMc0ZtV0F1MW5aUDBjYXlHNWw5RzQy?=
 =?utf-8?B?TW1UcGowazJpU2hTK0RjRFRhRUtLQXJuaTJ0RURmd3RxdGdiTlJWQThZYWhS?=
 =?utf-8?B?L0lFZDhBWHhRT1R0MnIzVzJKWE5NZC9IemhSZ2NyWDlhVVdsbW9uYTh4dlZk?=
 =?utf-8?B?N1U3WjJsVU9JdmhsM1BBVUhlbm9Ma2xrdFp1NXAvRTlINzVIQUVWNlVkVVNz?=
 =?utf-8?B?ZU5ZMGVnQmhLMWwyY3ppQWJVdS9TSGFzQ3JzdW9tc2E0RWdTVnhPQlZqWkxI?=
 =?utf-8?B?QkxqQmFYRGN4OVg4MUsweDVKQWtOV2M2NW5VWnpTczJpcCtzN0hFbnR1czFs?=
 =?utf-8?B?RnN3ckZKR2RSMlBQR3F0ZkxSMVFyY29CNW5LYXRkUDNIOWhRb3diTVNHMkR1?=
 =?utf-8?B?TTdySlBFVm1KTStjQ3pHSWNJZzdJZ0paQlNaZ0M4all1UmpzTGUyZUFINmgy?=
 =?utf-8?B?TStaaXhTbzhPR2JSZEF5UnowWVh2dENLd2QraGQreUY0WTcrQWVuU1EwdmJo?=
 =?utf-8?B?Z01wdGptK0llMzRvelpZZEo2WElndWx3T0pNYytlb1BXaml1VCtQR3UyTFpv?=
 =?utf-8?B?OW9zUklUdng3Rmo0Vm1iY3pvenVwYUZoaGVoK2FLbHFIL0Y2NHdlb3RhZ0pa?=
 =?utf-8?B?TEE0N2VJdTVWWDkwdklHUHZHYVB2VVk2YWUwZlh0SjM2cWZDVWtlbjk3cXpi?=
 =?utf-8?B?aU1WaGFUV1B3OFB6WHBOT2MwaWhpQ1NkNWFXTUIyd3EvTCsxdTFQRW9UT3V4?=
 =?utf-8?B?Z3N0YldVWXJoa2R4UFZ2YXVQUmRES2h0NVdWNWRwMXBUOWtNdWs5Rm1ySlQ5?=
 =?utf-8?B?bWIzb2trcUFFMVpXdEZZMTBtVEkrMmd6Q0FxWExyVUhNMm83NEZaNnY0S0F6?=
 =?utf-8?B?dDRCK3FrWWxkd1R1RXcwVGVXZGVKU2VESENVaHBhTS9QMmR3UW5MSFdoM293?=
 =?utf-8?B?dWdxajE5bWlGRXFLYmQvYXQ4T1pmQjZ1aUNvSWtjNGNpRUR2YWh2aU1UM0d1?=
 =?utf-8?B?ZTc1ZzlXR2JCRFFuZUQxbTFFT2hBVTV6NVpBVEVvM0l3Y0lsbHhYNXo0RERk?=
 =?utf-8?B?b095Wmt5TGJKMmpxTktwS0dRZTFGdWR2MlpvdUYvTWdoUWxMK2xtMFdwUnYx?=
 =?utf-8?B?VnJlcUVqMzNNUFFCcUxMektFbGd4ekViYS9aV3lqSzhpNEVDWEpWaHN0MHNh?=
 =?utf-8?B?ZGorY0ROdWxlUHZLWm1pOVNUbmpCTlIxWnZvYk5PYjZIU0cyQmExQ1lvc2Nk?=
 =?utf-8?B?SUNzaHN1RUdvTUM2c0QwZ1pzVXhpV29KaVJzWGZqbkVNanVTa2E0dXl3bTB6?=
 =?utf-8?B?SzNkZmtYRTJWYnZaVnUrWDhLL0lUNkMybi9DV1M1Ym9wTDhKNGp2VG1JNVNt?=
 =?utf-8?B?M0sxQXM5aXphTVdTSXdIRkc1QkVNM2VjQ0J1ejBXdEtFT0Fod2dmNU81Y21y?=
 =?utf-8?B?T0luMGgyNU44UEFoU2tPU3JjdnZ1bGRSYVJLMys5MmQyNlU1MktKMmZHZUwx?=
 =?utf-8?B?Wjl4NWxGbi9ldW9YeERGamRsOTY3Smhua2o3QklGVXVzaXFOK0M2VEx3emdQ?=
 =?utf-8?B?ZTEyQVAzSFJFMnlyTCtLamN5d2dNZk9FQ2VVV1FaODdMdmorcHhMbjNwZzBB?=
 =?utf-8?B?NE9YN0ZEaU1LdnJjdkNwMXJtYVB2Mmc1Snd2TUVrNFlENXFrSE9LL3lHLzhJ?=
 =?utf-8?B?Umt3ZmRYWm9aakxqWUkwTzBSK2g1dU1aNWcwVzB4VkVnckh4WWFyNnM2YmUv?=
 =?utf-8?B?Z1cwZ2VMVkRGZTFQTHZOUmY3cGhEQmFvUHRaUXAwU3VRNG03bU9RMHB1VVFz?=
 =?utf-8?B?WEY2YlFITkR5NkVDdDhqbWIzNG5DcXoxWlFRYWpGQjN1YlhUMFNMVElSNlJk?=
 =?utf-8?B?clNNLysrS3VBR3BHdTNBSzRnMnRZVlV2ZEErZHhoWFRiY2cweXdUS09iRkEx?=
 =?utf-8?B?WG91NUtMS0lEWEc1MEhiV3RtcjlqK093Ui91SFMyaEE0ODVYSjByUTBCVE4v?=
 =?utf-8?B?NWsxVkk1RTlWdDViY1F1RUh2bTNhdzRoZ2dvZEVvNndRUnZiMTZLNHhZalJu?=
 =?utf-8?B?eExoRmdIUElVcmNiVG1wdCs5TzBSbzFVZjY2MTZJcWxoSEdBMW9BZ0ljOVE5?=
 =?utf-8?B?TmRyME9xdDVTUlhTUU1xdTdMM1h3UkU2dlliRmprcDVTN29zTzhucXJFRUFp?=
 =?utf-8?Q?XHmXj2LKbUgIW249gqufHHw=3D?=
X-OriginatorOrg: amperemail.onmicrosoft.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 626fb682-a410-4c40-f603-08dc18bca2e8
X-MS-Exchange-CrossTenant-AuthSource: PH0PR01MB7975.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Jan 2024 07:02:44.8603
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3bc2b170-fd94-476d-b0ce-4229bdc904a7
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: l2dgaVkCykNwQiGwp025k2ge4hFVNFjJNnLlXwZoqoB/F/2vulqsKgRPEIZ1srUXIf13/F7EVYJUw61xgN5KBGZPz56eDwyMrDSeN2ZpGzdLpiQAaLs9coK5z0wu9WFG
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR01MB8240


在 2024/1/19 14:46, Shijie Huang 写道:
>
> 在 2024/1/19 12:42, Yury Norov 写道:
>> This adds another level of indirection, I think. Currently cpu_to_node
>> is a simple inliner. After the patch it would be a real function with
>> all the associate overhead. Can you share a bloat-o-meter output here?
> #./scripts/bloat-o-meter vmlinux vmlinux.new
> add/remove: 6/1 grow/shrink: 61/51 up/down: 1168/-588 (580)
> Function                                     old     new   delta
> numa_update_cpu                              148     244     +96
>
>  ...................................................................................................................................(to 
> many to skip)
>
> Total: Before=32990130, After=32990710, chg +0.00%
>
>
>>
>> Regardless, I don't think that the approach is correct. As per your
>> description, some initialization functions erroneously call
>> cpu_to_node() instead of early_cpu_to_node() which exists specifically
>> for that case.

sorry, I missed something.

I am not sure if  the early_cpu_to_node() works on all ARCHs.


Thanks

Huang Shijie

>>
>> If the above correct, it's clearly a caller problem, and the fix is to
>> simply switch all those callers to use early version.
>
> It is easy to change to early_cpu_to_node() for sched_init(), 
> init_sched_fair_class()
>
> and workqueue_init_early(). These three places call the cpu_to_node() 
> in the __init function.
>
>
> But it is a little hard to change the early_trace_init(), since it 
> calls cpu_to_node in the deep
>
> function stack:
>
>   early_trace_init() --> ring_buffer_alloc() -->rb_allocate_cpu_buffer()
>
>
> For early_trace_init(), we need to change more code.
>
>
> Anyway, If we think it is not a good idea to change the common code, I 
> am oaky too.
>
>
>>
>> I would also initialize the numa_node with NUMA_NO_NODE at declaration,
>> so that if someone calls cpu_to_node() before the variable is properly
>> initialized at runtime, he'll get NO_NODE, which is obviously an error.
>
> Even we set the numa_node with NUMA_NO_NODE, it does not always 
> produce error.
>
> Please see the alloc_pages_node().
>
>
> Thanks
>
> Huang Shijie
>