Received-SPF: pass (google.com: domain of linux-kernel+bounces-30925-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <b8786c38-d6c4-4fea-a918-ac6a45682dba@amperemail.onmicrosoft.com>
Date: Fri, 19 Jan 2024 16:50:53 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] NUMA: Early use of cpu_to_node() returns 0 instead of the
 correct node id
To: Mike Rapoport <rppt@kernel.org>
Cc: Yury Norov <yury.norov@gmail.com>,
 Huang Shijie <shijie@os.amperecomputing.com>, 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, 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>
 <Zao13I4Bb0tur0fZ@kernel.org>
From: Shijie Huang <shijie@amperemail.onmicrosoft.com>
In-Reply-To: <Zao13I4Bb0tur0fZ@kernel.org>
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?WWFzRU13N202REFWU1RMTkRMNHRpSEtzRVdMVVFFcVkyMUlVNDFOamdqUzdh?=
 =?utf-8?B?U3lTVFRMRE96ekFaQzVzaXQ0c2FHcGdkRkRTYld5UHFZQ3hZbVc2eFREeCty?=
 =?utf-8?B?SVNvcktKZjM4WU1XbzlJS2VpOGYvalcyT0tGdXVpYU1NdXdGSHZDaklJMGVz?=
 =?utf-8?B?THovdk5ZcDdwUGJqYUxCV01ITTFETU1jQTNTYTlsRldBekhHNjgzeHNQMUli?=
 =?utf-8?B?Mm5RdE5BUXdxYmZ6eEZ6eVVkUUFLR1lIdjZySW9VWHhlT2Y0dnFXUzNvNlVQ?=
 =?utf-8?B?ZFVDSU9neDA0eC9neXQwdEF1aEdZTUc1Um5vaVYraCtCckFoKzc1c2hXOTU1?=
 =?utf-8?B?SXFZMGdwUGtDR29pOXJ3TEEwTWhOOGJJcitFT2hkZGI1cnJtU1VEaXhMTWND?=
 =?utf-8?B?QmpLSlpKNXhxT2lSTVhCUjd3NnB1QVdVbjVXSDJrSlpMb2JkK0o1d2s1ZXFl?=
 =?utf-8?B?dkI4dmVDN2QyQWZ1dTNneXUxWU9jOUZLWGpEaUttZUJxZ2xheldsVmdSdU1N?=
 =?utf-8?B?VXZkTklRSDcrUmQycnVXVUhxTlNSUVc1T2hjeHp5REo3cmFuSGZQajZ0VzNt?=
 =?utf-8?B?N01hTzd3N0JZZ3VGbUl0WE1aQ1RMbDA1VktHUXdJSmFUZzc1QkxvTUhsVEVS?=
 =?utf-8?B?VFNleW9aZlJjemVlVElDVHJrdGVSaU1UeTU0ejRnM3dJQjhzSHg2eWtMb1pV?=
 =?utf-8?B?bXcrU1NYUmNQZU9nLzZXRm9kd0ZpVzBwRTRvMXVodUVma24zaG0xRjc3blhQ?=
 =?utf-8?B?Q0xOUnhLcXphTjhaRCtVNVdXWVRHeUZnZWlVQ1o4aHV5elg3QTdiZ0QrempH?=
 =?utf-8?B?STZKWGZjM0tqMTdGUUM2LzY1QXM4Y09oREZISlo5RkF3aXFMUy9taXBRb1Vi?=
 =?utf-8?B?TWc3TnFLQXF3WmVPemdVb1VKdlJFOU9oVEpEMysvTzJ1Q1ludWRXZGxkRzR6?=
 =?utf-8?B?dmY4L1hQTWJRcURXdEZNQTZDclNEMmRCVHBUVFh1Vjk3bzJTVjV2Z2ZKKy9D?=
 =?utf-8?B?eDNtVmhKa1BjT1o3WWJUZzIxZy84d3dRMGJhalFKREFWNmlKdWw2dmVkdDVD?=
 =?utf-8?B?YmF6VjJoYmU5clduaTdmK09nTHIrSTRNdCt1d0lvOFZnQXdXVkwvYW9TanU0?=
 =?utf-8?B?eDZCYUw2SG9SbFM1K2lNTkRtKzNiaVpORk5NNm5wM0FXUVdSMUlFMlhQSzBI?=
 =?utf-8?B?b2lZcTlXdkFQV1NjUE1QRzFuWWxuck1WMFl6UHB6YzRNblhvWVIza2JxeHRz?=
 =?utf-8?B?OXA4L2E1dCt6eTdhZHBGaUhKcERteXpuMlo2VG9uQmFieTNvNTJFZStVUDdI?=
 =?utf-8?B?SWx3OVBXZ2FMb2VXZ0hPL2gvV0p5S0hoMVAxajBPTXV2cmRjLzYvaUs3Y3ZY?=
 =?utf-8?B?ZmRIUEZBU0xDa0ZzeHBCZzZkYS8xaFd1YStnU2c5NUU0c0ZpWXNWdG01SlQv?=
 =?utf-8?B?RlMzOG44eFNQbkVkTDBxekkrL3lRcVVpYUJSYVlNN0xPK0ZBakJzWVVRbXNS?=
 =?utf-8?B?UGp6dXN6YmcxK3RmZ3R0ZlRVOS9OekluVDBTN2hIa282bjRnbEk0aGMwb0dx?=
 =?utf-8?B?U3ZQVDNTZFBWTjVXNVg5WDhIdlFSUVBsTUlaYnlVNk9LYjkwbEFacG1aQXMy?=
 =?utf-8?B?UU4vZFZKMDJ6WDRIZmkwOGY5V0VWOGc5aEJHMHZIMGtmaWpYcjNMbkcrdnQ5?=
 =?utf-8?B?aUJYcll2U3VYZXFNV1ZTTnd6Z2FiZ2xNMXFZYUtVbERZUy9aRGhVNHFaMS81?=
 =?utf-8?B?V1NYaUdKWllDQ2VGZnZnWGNQVElhbml6VHJTTmJxVFVid1NvUzM0cXVXa3Yw?=
 =?utf-8?B?UUhlcVFtRWNGSy9NNUd1Y2JlQnRZcG1vSnBLWHViVExrWkkvVkJzRTlBZFhq?=
 =?utf-8?B?bDZxSVBVUHNPbE4rVEtFU3hRWXNrMjJ5Z2UxKzlaSE1SckREM3BNYzh3QnRG?=
 =?utf-8?B?K1ZhSmdoZ1FMbmppeThwRE8zRm9od0NMb0l6WG9DUkVVRlp5b2hnYS9QNGpm?=
 =?utf-8?B?VWJBRzc0MDJMclRzTjVlZnBhSnFrNElJNTNVeGc1N2I1a3VBRHZHVFRob05M?=
 =?utf-8?B?bDNKcmJZY2Vqa2lDUVk2a29mT21BM0k1V3FodnNiMU0vdi9pQ2owMEFseUph?=
 =?utf-8?B?dnJJVlNRd3l3VFVrT3pJTGsrZmttaVBhemE5WjNrK1dzaXUvWUg4Z3ppWStK?=
 =?utf-8?Q?eO7bIPr6cgVldQDrJWm/mA0=3D?=
X-OriginatorOrg: amperemail.onmicrosoft.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 43278f3c-1f53-4962-2382-08dc18cbc760
X-MS-Exchange-CrossTenant-AuthSource: PH0PR01MB7975.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Jan 2024 08:51:08.3731
 (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: 1oSkGiYmlqXlfiQazflNhxShN1MZpL3lSG9oeuX8F0LigO0LkyeKwCUxRAJzZPGsKAAQGTKHcmUGglVxzjpAPcwdysF2MncRR7KbIWYT09Ys9hCJtDaOXQkF8sox5ifU
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR01MB7567


在 2024/1/19 16:42, Mike Rapoport 写道:
> On Fri, Jan 19, 2024 at 02:46:16PM +0800, Shijie Huang wrote:
>> 在 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%
>   
> It's not only about text size, the indirect call also hurts performance
>   

The cpu_to_node() is called at very low frequency, most of the times is 
in the kernel booting time.


>>> 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.
>>>
>>> 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.
>   
> Is there a fundamental reason to have early_cpu_to_node() at all?

The early_cpu_to_node does not work on some ARCHs (which support the 
NUMA), such

as  SPARC, MIPS and S390.


Thanks

Huang Shijie

> It seems that all the mappings are known by the end of setup_arch() and the
> initialization of numa_node can be moved earlier.
>   
>>> 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
>>