Re: [PATCH] mm/mempolicy: Fix redundant check and refine lock protection scope in init_nodemask_of_mempolicy

["zhen.ni" <zhen.ni@easystack.cn>]

Hi David and Gregory,

Thank you for your detailed feedback and insights. I deeply appreciate 
the time you’ve taken to point out potential issues and to clarify the 
behavior of current->mempolicy under task_lock().

Regarding the "redundant check" in my patch, my intention was to 
simplify the readability of the check for mask and current->mempolicy. 
Your suggested alternative:
	if (!mask || !current->mapping)
is indeed more concise and clear. I fully agree with this change and 
will incorporate it.

On the issue of reading current->mempolicy outside the task_lock() 
context, I acknowledge the potential for introducing a race condition 
when current->mempolicy could be dereferenced after being freed. This 
was an oversight on my part, as I was primarily focused on reducing the 
lock scope.

Regarding the potential performance improvement, I recognize that the 
optimization is minor, and in this specific context, the lock is held 
for such a short duration that it is unlikely to provide any meaningful 
benefit.

I’ve revised the patch to simplify the conditional check and added a 
comment to clarify the behavior of current->mempolicy based on your 
input. Additionally, I’ve removed the lock scope optimization attempt to 
avoid introducing any potential race conditions.

 From 073e4ac5ee6a3f2b45804492f3865cf9157155e2 Mon Sep 17 00:00:00 2001
From: Zhen Ni <zhen.ni@easystack.cn>
Date: Fri, 22 Nov 2024 11:48:05 +0800
Subject: [PATCH] mm/mempolicy: Improve readability of NULL check in
  init_nodemask_of_mempolicy

Refines the readability of the NULL check in init_nodemask_of_mempolicy.
Additionally, a comment is added to clarify current->mempolicy.

Signed-off-by: Zhen Ni <zhen.ni@easystack.cn>
---
  mm/mempolicy.c | 8 +++++++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index b646fab3e45e..0f0dd33d20d4 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2132,7 +2132,13 @@ bool init_nodemask_of_mempolicy(nodemask_t *mask)
  {
         struct mempolicy *mempolicy;

-       if (!(mask && current->mempolicy))
+       /*
+        * While current->mempolicy can race with someone changing
+        * current->mapping, it cannot race with changes that set it
+        * to NULL. Such changes are restricted to specific contexts
+        * (e.g., process initialization or exit).
+        */
+       if (!mask || !current->mempolicy)
                 return false;

         task_lock(current);
-- 
2.20.1