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@xxxxxxxxxxxx>
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@xxxxxxxxxxxx>
---
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
