Received-SPF: pass (google.com: domain of linux-kernel+bounces-30828-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <1cd078fd-c345-4d85-a92f-04c806c20efa@amperemail.onmicrosoft.com>
Date: Fri, 19 Jan 2024 14:46:16 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] NUMA: Early use of cpu_to_node() returns 0 instead of the
 correct node id
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>
From: Shijie Huang <shijie@amperemail.onmicrosoft.com>
In-Reply-To: <Zan9sb0vtSvVvQeA@yury-ThinkPad>
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?Wk1OR1hhSFA1L2tFZS8yN012Ykx5Q0ZpbE96N3pEVHg3ZEhPSTl6blFkSmFy?=
 =?utf-8?B?WGJlYngzQm1wL0NzRVFJQjN5R2hZckpJOEM3dXJTR2ZUV0tBcHlGREtyajc0?=
 =?utf-8?B?NkR3dEVuRU9lZHZxRVd2YkFSaFJIK0w1RnpOTTZhbVdLaHRLanRVd1RUemQz?=
 =?utf-8?B?VXIwMnlBcEk3QlIreHkxQnNVeDZ3dkJNZzlIVG84REM4Ni9EWkRCNy96V2t4?=
 =?utf-8?B?dE5KZ1ZvOVRwVzVVdk1qRVFDZG5tNmJ0T3pjMEF2dldBRVEwTmRsWmxBNVBu?=
 =?utf-8?B?ZkYxUDQvWVVta1NmOFRaU1FhMWZrV3ltRHN0WlpHL1N1RjNweE5vUnVJM0la?=
 =?utf-8?B?cUpaR3NlZld5SXowMWYwdnFRbEhWVm9sY3Z5UHRPS1AxUVZBTmxac0c2UGt2?=
 =?utf-8?B?eDMyaEVSVHg4aWxwNVFOQS9QTDVHY243NGhLd3BzZHlWRVlsMnBIVmU4RUdi?=
 =?utf-8?B?Wm9yOUQ2b1V1Z1B2T3VENnZLVHNRRWlaOER1cUN6QThJSzF4YXRXUVU4L29r?=
 =?utf-8?B?eE5KWi9iUkJsU1BtZHF1SXMrdk1uc3pJazJhVmRNMDhlVkxFOHV2b0sxSWJt?=
 =?utf-8?B?WGk1aGNYS3lCMStKaWJRRG11RllyM2dJckptYnh6ZlVSMHZkQ05ic0w1OHNh?=
 =?utf-8?B?SWI4WDVhakFYZFZvTmFTU3p2REg4R3BBbnZncVJhREJoYnpjU0R6Qk00a3Ro?=
 =?utf-8?B?RUkvNjR6WHJVWXhpUFJ4VWNjOEpnT0lTbWpLVENJSzczTll0MldvTjQzQ3dL?=
 =?utf-8?B?ZVVoK3AzRkloUlVGWkhkMDgrcG04Z3AzMFkza21jb045VTZSUGlpVldvSnhk?=
 =?utf-8?B?Y2NHQ21SRDlxOUFxU1d6WTdMdWNZMGxQaGxtQml5eFYzdi8vY1pwRi9sR2Zq?=
 =?utf-8?B?QXFiTkQyU3Fvb0hkLzc2eVc3YlU4UW03RGdJQTdtUlZWSUtkYVQ4OEJCdklL?=
 =?utf-8?B?L1FTVUl4VTc0cTEvRXByS3lOcDV2OEtxMlJ3a1J1a1ZwYi8yQzYyTkJ2c2dq?=
 =?utf-8?B?MjYwWmszY1oySlhDRGtKaFN1dGgrQlJRU0RZa0lwQjBxMFNFbWpHV0h3M3ZB?=
 =?utf-8?B?TkJYdFM0U29pUGxZcmFpcmoxWUdaVldoa3BURE9yUjhLb0pTUS9TQXgwS3Zs?=
 =?utf-8?B?TGJBUDVYNmNwdWVOUGFVbWZjUGdrNEg1eWNTTnEvb1FTbjZrZllhd01ROHhp?=
 =?utf-8?B?MWcrNUhJVUROOXlGbk1iOEowWVg5STZZekJPd1BUajZrZHd3RC9zdE02bWVh?=
 =?utf-8?B?SVc5N0tVbG01eVlmajAvK3VlL1ZkZXM5akFaVlFjaGo0Mk96ZEVpRXovNDZ2?=
 =?utf-8?B?N0Z5MFNxQkVFMGc1WU1BK1FabEFPSlI5U1k3c1NjSU5vT0NPQUhUSHFsSXY0?=
 =?utf-8?B?YnVIMTlaUm1jZHZUeERsU2JwWG9xMi9DaFA5T2hhUjM4UUt1cllkK1Zza1Rk?=
 =?utf-8?B?MVo1TjJVcmZNbyt3SDdxNzhyR2x1d3lFVEdNTVRZdXZ3ODd2eG13cnI0dUNC?=
 =?utf-8?B?d2N6TmdzcnVnZmx4Ym85YU1zMDBJSzVRNmRwL0xiNXZxdHFKQTcyWEhqNDBX?=
 =?utf-8?B?S0lwcVZ4UWxwbUFQZHMwNnMrWUNmSER5dnJ5YTFNN01iTm9QMHRrMVBjeFhJ?=
 =?utf-8?B?YmxYMURrSlRWOVlvd1JvakIvZlE1cDdYYkZBdWcrOEcxTnJLdVZhV2MxRmIy?=
 =?utf-8?B?YXV4dGRIcXdBV3lXOTNuMmlPZS9NR3VHL1NyVFRwUG1mSWVTRjBPSGVuM1Ev?=
 =?utf-8?B?dnBvaDFPa0F1Tks0VUdqU3Q1U0wrR1FHT3o4bVhIQjRUY2Uza0Zta2lzVHdR?=
 =?utf-8?B?NFlPNlA5dTUwMVIxc0crVXcyQlRQa0JMbXhJSlNlR3kySmkyL3FFdjZXVlRi?=
 =?utf-8?B?QUlFbDJWeVlsMFNqaXFtVjhQWFAvNE9ESTJaRkx5MlpDak5RaGEwN2VTYzk5?=
 =?utf-8?B?RjRXZnJ1TmNIZjV0dVM1MmNPUHZlMmVRWHQ4Z0xCWU1YUnY2cXRGWWhnSWFr?=
 =?utf-8?B?c1Y1NmNmQit1aW5VNWJOMFZtd3E3YUR1cVlacUNsM2RneVBUeHJhQTh2VHdO?=
 =?utf-8?B?aXRuLzRtYUFqM2Y3YTc3bXNoZzhwcGZnS3BQMzhqaGZvK0JRc1NRN3p6RDFl?=
 =?utf-8?B?aXh1eDMvRjI3b2ZGYWVKaTI1Q1hmQjB2OTl4ZDlSN0dNaTkrM1Fub01laUNo?=
 =?utf-8?Q?B20T2V3cGUZIlLJgRJacLEQ=3D?=
X-OriginatorOrg: amperemail.onmicrosoft.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6a71be4b-4d28-4475-f9fb-08dc18ba5e97
X-MS-Exchange-CrossTenant-AuthSource: PH0PR01MB7975.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Jan 2024 06:46:31.2149
 (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: aL3NMB/llKCtTPp63balqZ42Tl4RRBeAJeirXOZ7LmljgdrDW1oFiM1OcbTxQDQK51s68scd4lSctql53pA6PiB5e9R2H2Nsh+LtUUKfylp/AJ3mCRjqaIT1Or84w08p
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR01MB7291


在 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.
>
> 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