Received-SPF: pass (google.com: domain of linux-kernel+bounces-29291-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Wed, 17 Jan 2024 12:46:03 -0500
From: Gregory Price <gregory.price@memverge.com>
To: "Huang, Ying" <ying.huang@intel.com>
Cc: Gregory Price <gourry.memverge@gmail.com>, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org,
	corbet@lwn.net, akpm@linux-foundation.org, honggyu.kim@sk.com,
	rakie.kim@sk.com, hyeongtak.ji@sk.com, mhocko@kernel.org,
	vtavarespetr@micron.com, jgroves@micron.com,
	ravis.opensrc@micron.com, sthanneeru@micron.com,
	emirakhur@micron.com, Hasan.Maruf@amd.com, seungjun.ha@samsung.com,
	hannes@cmpxchg.org, dan.j.williams@intel.com
Subject: Re: [PATCH 1/3] mm/mempolicy: implement the sysfs-based
 weighted_interleave interface
Message-ID: <ZagSW5TXzZeKErlW@memverge.com>
References: <20240112210834.8035-1-gregory.price@memverge.com>
 <20240112210834.8035-2-gregory.price@memverge.com>
 <87le8r1dzr.fsf@yhuang6-desk2.ccr.corp.intel.com>
 <ZadkmWj3Rd483f68@memverge.com>
 <87o7dkzbsv.fsf@yhuang6-desk2.ccr.corp.intel.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87o7dkzbsv.fsf@yhuang6-desk2.ccr.corp.intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?j0cmSXBYdpFL0l1eBzxGyr1XropQu3o1m44KoyQCRKxJhXCqEERpBz2Lf2zF?=
 =?us-ascii?Q?zzVobL/l8p+O5J6UZurGDDfQx2bWdnofF1ZYn+InMCMjJaWkiAmAItzhk7OD?=
 =?us-ascii?Q?6CI0CsdsOAyXJz30MI/xJCAPg903LfBOGcL/Kvu+vhQvGQjekAHdwZg+cKr8?=
 =?us-ascii?Q?lQMrsXv/STxlU8Sz6Zd8KAKFfQIYGNwi6G6WKl2cDXzI7ZXJX8QfKrhGHFjZ?=
 =?us-ascii?Q?ore/+K0cQjNHXJeQIexb9jFuAWQplAgwgHS4+59hwQvRkzUaFfKlLEvSc9yf?=
 =?us-ascii?Q?7wVVj0jFMfyxn16FBx5KuxXdvv+g7Ip+ENkx5H3YP6FKqVULz4qA0eK2LacA?=
 =?us-ascii?Q?NuAugLI18u1b3tkZ22nWb8Qt1PdrdEkXt10FW+uzBB28IfAMSXlP68gZIzHA?=
 =?us-ascii?Q?y3bZ71kaM3wk51l8VX6k49KLUwgCM49eliuYI+GslvDAs/RoAhWakPcseLHM?=
 =?us-ascii?Q?kLTWDFqvb6dc3KQW56f3HbWTy3OYA3PWXszj/RsDvgYbXiONXWHKckQyUwAA?=
 =?us-ascii?Q?tdS/puKeYrKwdHAvsrKI53+djsAwxkho4Q9iFwnAJmLwxJ5hkUgr+5Q5U/j9?=
 =?us-ascii?Q?bwp1Yoqip25KWNjoIuGqVlyJ+uq4NhrxZRdidfdOXan6Tt9HiMCDc/Q7Zfbo?=
 =?us-ascii?Q?X/JELV7OFLSXNPkuuzmjTW019pTngEAtm+HT7ZBOfOBkxqKpSavYP3tkniOL?=
 =?us-ascii?Q?87EIJLrP94nleV7IjVaNVGmoyxRO+n07ezwujft0ZQLCaOikNDuQkw1lqVPm?=
 =?us-ascii?Q?x+DYcDoAwfdnGSt1pS+8LvjpMIIgPteDgC7a07e4wpRzFkNBK+Jo1KjbsgJe?=
 =?us-ascii?Q?+SUC4641n1CTj2BiVX+kWfrCu/T9iJ84Qd+8rKEcPCe/SdzCa4N2miWzyh54?=
 =?us-ascii?Q?YvduN7/CM+37alsEV4UHHuLx5/FA8wscBvaLSVq7q5WCdsq9PolUGKCINWzN?=
 =?us-ascii?Q?UWX05t3Vl/gdjeSc1SrhDxAD1d+9qcvN0w01IDFcXXrJpjZvuWz3sG4LU3vB?=
 =?us-ascii?Q?CyP0TVr8ELGmdkyrAfi033LJFq0ItmxFpnDKqy9XHroRkSlDPf7tO3wozAVU?=
 =?us-ascii?Q?6KM4/cwT18r4uW+9Ox0cyp2Z+RvVrrUJ/GM7BlXrWCJHaqdpPmtzOKFgEVQh?=
 =?us-ascii?Q?v1KxkaCE7eLjfMQJw8l+8zggVIUfr0BFcsHdogyKIRJeWOdcV45oJfuZiiGT?=
 =?us-ascii?Q?4NPu15zS3jlrTquQGg3q90tlDFsQoLDwPzRfOx4zlqzAoOTrP7tkVbQJ2meL?=
 =?us-ascii?Q?o9KCXqpetQxOHFpt93JJ09j3yLB/RlDDAWBnXDDEFCX9JUCerzzvfu04YQ1S?=
 =?us-ascii?Q?v5MfQfpC+oX0zecm+R7mu1FD2dLp0te/DZpOCw6xcOL/LI45/5l0FJghoLIq?=
 =?us-ascii?Q?Umoh5wkXV1+YgcJST2g0yqH7MaYkEqe+j6B99hqrjxiKtos4CPFIxCPZUCwj?=
 =?us-ascii?Q?XvMqjUmoS8qDqjjUL0FM7r2m00mOB7TB9H4selbHq87x2/LIA1wRZdcLFBI3?=
 =?us-ascii?Q?TZ9nLovvauyB8th8l+/vVJZpGXtEHFj/vXla7CT1oGvqWz/PRckyi1wuUFY1?=
 =?us-ascii?Q?E6vTcPKlxoxN0+ccLj/wYv+Rcdu1ek09qvnF0ldEoFAMot0I4kehWOXKzBLf?=
 =?us-ascii?Q?ug=3D=3D?=
X-OriginatorOrg: memverge.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0fd1f319-69f4-4600-b5ec-08dc17843250
X-MS-Exchange-CrossTenant-AuthSource: MW4PR17MB5515.namprd17.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jan 2024 17:46:12.9083
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 5c90cb59-37e7-4c81-9c07-00473d5fb682
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: U91Xgsj2DB2IO3ZT8ALfEpxRlsvOjNkJGn90KIv2hVqcX8henYzCaeSLpTTZT9q9exnXNWejLbtb3wppMHAmYVF4Y3M7hIjO9CiW01VpTFc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR17MB6890

On Wed, Jan 17, 2024 at 02:58:08PM +0800, Huang, Ying wrote:
> Gregory Price <gregory.price@memverge.com> writes:
> 
> > We haven't had the discussion on how/when this should happen yet,
> > though, and there's some research to be done.  (i.e. when should DRAM
> > weights be set? should the entire table be reweighted on hotplug? etc)
> 
> Before that, I'm OK to remove default_iw_table and use hard coded "1" as
> default weight for now.
> 

Can't quite do that. default_iw_table is a static structure because we
need a reliable default structure not subject to module initialization
failure.  Otherwise we can end up in a situation where iw_table is NULL
during some allocation path if the sysfs structure fails to setup fully.

There's no good reason to fail allocations just because sysfs failed to
initialization for some reason.  I'll leave default_iw_table with a size
of MAX_NUMNODES for now (nr_node_ids is set up at runtime per your
reference to `setup_nr_node_ids` below, so we can't use it for this).

> >
> >> u8 __rcu *iw_table;
> >> 
> >> Then, we only need to allocate nr_node_ids elements now.
> >> 
> >
> > We need nr_possible_nodes to handle hotplug correctly.
> 
> nr_node_ids >= num_possible_nodes().  It's larger than any possible node
> ID.
>

nr_node_ids gets setup at runtime, while the default_iw_table needs
to be a static structure (see above).  I can make default_iw_table
MAX_NUMNODES and subsequent allocations of iw_table be nr_node_ids,
but that makes iw_table a different size at any given time.

This *will* break if "true hotplug" ever shows up and possible_nodes !=
MAX_NUMNODES. But I can write it up if it's a sticking point for you.

Ultimately we're squabbling over, at most, about ~3kb of memory, just
keep that in mind. (I guess if you spawn 3000 threads and each tries a
concurrent write to sysfs/node1, you'd eat 3MB view briefly, but that
is a truly degenerate case and I can think of more denegerate things).

> 
> When "true node hotplug" becomes reality, we can make nr_node_ids ==
> MAX_NUMNODES.  So, it's safe to use it.  Please take a look at
> setup_nr_node_ids().
> 

~Gregory