Received-SPF: pass (google.com: domain of linux-kernel+bounces-43031-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Mon, 29 Jan 2024 10:48:47 -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 v3 4/4] mm/mempolicy: change cur_il_weight to atomic and
 carry the node with it
Message-ID: <ZbfI3+nhgQlNKMPG@memverge.com>
References: <20240125184345.47074-1-gregory.price@memverge.com>
 <20240125184345.47074-5-gregory.price@memverge.com>
 <87sf2klez8.fsf@yhuang6-desk2.ccr.corp.intel.com>
 <ZbPf6d2cQykdl3Eb@memverge.com>
 <877cjsk0yd.fsf@yhuang6-desk2.ccr.corp.intel.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <877cjsk0yd.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?CeuUms+e7X+eFBk+MoNuTCY6oZj00nJnkDfuYDldeKdXQT+Mt4sRnh8EPAzq?=
 =?us-ascii?Q?I9xhKd5uKHdCwiPebmwKAvUi2Aef8CKKb/7AYbigNTh0aQHhs4PHteuM/nzo?=
 =?us-ascii?Q?KPcF1Putt0Byd2qpWyOu2XCmLrqbJcWsnu5PPgg5qRDPPAn+lUmDkm79dCps?=
 =?us-ascii?Q?IeSuhNSTUCfNPhXIi+GOYfChetUnYCLgLyxnCeTO5zaxunhD2wGOG2RMaIFo?=
 =?us-ascii?Q?XpChrxRVZafpYjXmQpHVuK5yuYRN5vpLiWp2ZRXqJfSsj9KEpCBl3yL2oDEd?=
 =?us-ascii?Q?tlQYI0d92twUzu+LLICS92sCpQUk4BclOulxTgBKtcl2wiAuOD0RNMe0RXwj?=
 =?us-ascii?Q?FZ1A5oKm9r9qr0B2BCrWoj7m41lx90GZAGWvjOShUzg7aTvIkG3Nl/ic4bk3?=
 =?us-ascii?Q?F2hEshtnVz6clk5DfbpC4aqIkJ8tlYPh2e2atzajJlbFZeoAZpPlIIkHk6Zl?=
 =?us-ascii?Q?rZr7wi3bSsm5HV1sHKcV7syCc4Zifqrfp0S4CzoXPxXya/4hcNeiVi2IOGGY?=
 =?us-ascii?Q?Hwmq1U3EQDgDrB8Vb6ipJ4y6qXIIlTZIuMjQsyQMw69F3x1MPH5c2LPYS36P?=
 =?us-ascii?Q?M9XS3cE/D8FzCqTKFwHha8d7s6CB/Xz4iCwtG+0FcWgIhFVmyuQuicwFjOM2?=
 =?us-ascii?Q?ffK6LALB35X3tirl3ycG0MECk0zYYwwqcpBrD3tMDRGPf1pm1QrDdixZEEFg?=
 =?us-ascii?Q?BvkdePgDatugXXKtCMiK3jssR+crNPlLNG5C3NDktmt63vdSQB6ykuCLyYDU?=
 =?us-ascii?Q?UnxLNmFI0/dXkjXYUpM1jj9+x2+eCNeq6NcCcpw0PmikHEq14s42boKtSNgU?=
 =?us-ascii?Q?RByRw4YSy5g5sClN8bFsRCEoSjO2rbaTSF/TGJlWD4m2GZu4almb3TxRDBzt?=
 =?us-ascii?Q?a97GQ7aVJsTA1qozNNTv97m6TBQKAVo2lmzcZBDtR+KCP9jVHd8V2E1/lkIi?=
 =?us-ascii?Q?eb2jYB/V3QaHTuMrZnb0B9d27lIfrChDo84u4q6io/l0JG5wr3/mKbRY16xh?=
 =?us-ascii?Q?1lN1Ow9lYhTTPnaMhQgk4Rd1t9698PnBcpSqqDHksG/t0APwCSKygkOF+Jnb?=
 =?us-ascii?Q?OialfFzKlQDnoO3pt6bCRlOvpzNGOldSschRS8k7Vnib0iCmEL3iP8oQ35a3?=
 =?us-ascii?Q?TqGk5wMzdfARzHkFRBkIBI+zvkWE+D07SoF3FNhW3XeB+mslHqqzlPCdOzd4?=
 =?us-ascii?Q?vC2gqKD2iPC2s1gftZ2s0UQzp+ry8Ed+gHwPDF6xG6hcpZQ4HHDQMjs5h+l7?=
 =?us-ascii?Q?XjZvBdNDLGIm7Ooj3dtEbhrr7L14tDP6X+KEu7l5hZWdzXNLUITeW4EtPR/j?=
 =?us-ascii?Q?hZgnaaq8gORZpIMzdMwPaLvy7Hs/WqsNzBwxYmmTRJgN5AlRWt/Cs2hYuaww?=
 =?us-ascii?Q?mcprfHAIVDRq/t7wlD9kJxP5lqpBnmzW+aMhs6FAebXCxo1fBiav+qPNJz5+?=
 =?us-ascii?Q?cJrofY3TntvZp9oMX1ATjdqpfN+Yo6RtXETaHCnIcX5N0zRhNgThZs9gKyfU?=
 =?us-ascii?Q?CqDWG97RjDxWEIWkfqKpE+CDJaXRLYeiJDZfD4xTeLGNXbfFI2zFi1bIzpZz?=
 =?us-ascii?Q?WE1badmujGsrg1wfd7jr/t+LuG96Up3/1SAiVhz3dOl/Ei4pw3K8QwwFBQTn?=
 =?us-ascii?Q?5w=3D=3D?=
X-OriginatorOrg: memverge.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8d86384f-02a6-4814-a6c7-08dc20e1ccaa
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR17MB5512.namprd17.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jan 2024 15:48:55.5292
 (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: zzy/a/eFjFCdFOetbcN0/zick0p7FcLxetEhpcRpiEwP+aNmDXnki+71EvYqbBeJGNN4JIxRAO3FRcwp9TOIEkk0mP+UX8RkWWMhlQvYYF0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR17MB4303

On Mon, Jan 29, 2024 at 04:17:46PM +0800, Huang, Ying wrote:
> Gregory Price <gregory.price@memverge.com> writes:
> 
> > Using current->il_prev between these two policies, is just plain incorrect,
> > so I will need to rethink this, and the existing code will need to be
> > updated such that weighted_interleave does not use current->il_prev.
> 
> IIUC, weighted_interleave_nodes() is only used for mempolicy of tasks
> (set_mempolicy()), as in the following code.
> 
> +		*nid = (ilx == NO_INTERLEAVE_INDEX) ?
> +			weighted_interleave_nodes(pol) :
> +			weighted_interleave_nid(pol, ilx);
>

Was digging through this the past couple of days.  It does look like
this is true - because if (pol) comes from a vma, ilx will not be
NO_INTERLEAVE_INDEX.  If this changes in the future, however,
weighted_interleave_nodes may begin to miscount under heavy contention.

It may be worth documenting this explicitly, because this is incredibly
non-obvious.  I will add a comment to this chunk here.

> But, in contrast, it's bad to put task-local "current weight" in
> mempolicy.  So, I think that it's better to move cur_il_weight to
> task_struct.  And maybe combine it with current->il_prev.
> 

Given all of this, I think is reasonably. That is effectively what is
happening anyway for anyone that just uses `numactl -w --interleave=...`

Style question: is it preferable add an anonymous union into task_struct:

union {
    short il_prev;
    atomic_t wil_node_weight;
};

Or should I break out that union explicitly in mempolicy.h?

The latter involves additional code updates in mempolicy.c for the union
name (current->___.il_prev) but it lets us add documentation to mempolicy.h

~Gregory