[PATCH] cpuset: Clean up cpuset_node_allowed

[PATCH] cpuset: Clean up cpuset_node_allowed

[Haifeng Xu]

Commit 002f290627c2 ("cpuset: use static key better and convert to new API")
has used __cpuset_node_allowed() instead of cpuset_node_allowed() to check
whether we can allocate on a memory node. Now this function isn't used by
anyone, so we can do the follow things to clean up it.

1. remove unused codes
2. rename __cpuset_node_allowed() to cpuset_node_allowed()
3. update comments in mm/page_alloc.c

Suggested-by: Waiman Long <longman@redhat.com>
Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
---
 include/linux/cpuset.h | 16 ++--------------
 kernel/cgroup/cpuset.c |  4 ++--
 mm/page_alloc.c        |  4 ++--
 3 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
index d58e0476ee8e..980b76a1237e 100644
--- a/include/linux/cpuset.h
+++ b/include/linux/cpuset.h
@@ -80,18 +80,11 @@ extern nodemask_t cpuset_mems_allowed(struct task_struct *p);
 void cpuset_init_current_mems_allowed(void);
 int cpuset_nodemask_valid_mems_allowed(nodemask_t *nodemask);
 
-extern bool __cpuset_node_allowed(int node, gfp_t gfp_mask);
-
-static inline bool cpuset_node_allowed(int node, gfp_t gfp_mask)
-{
-	if (cpusets_enabled())
-		return __cpuset_node_allowed(node, gfp_mask);
-	return true;
-}
+extern bool cpuset_node_allowed(int node, gfp_t gfp_mask);
 
 static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
 {
-	return __cpuset_node_allowed(zone_to_nid(z), gfp_mask);
+	return cpuset_node_allowed(zone_to_nid(z), gfp_mask);
 }
 
 static inline bool cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
@@ -223,11 +216,6 @@ static inline int cpuset_nodemask_valid_mems_allowed(nodemask_t *nodemask)
 	return 1;
 }
 
-static inline bool cpuset_node_allowed(int node, gfp_t gfp_mask)
-{
-	return true;
-}
-
 static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
 {
 	return true;
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 636f1c682ac0..0241b07d6f21 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -3831,7 +3831,7 @@ static struct cpuset *nearest_hardwall_ancestor(struct cpuset *cs)
 }
 
 /*
- * __cpuset_node_allowed - Can we allocate on a memory node?
+ * cpuset_node_allowed - Can we allocate on a memory node?
  * @node: is this an allowed node?
  * @gfp_mask: memory allocation flags
  *
@@ -3870,7 +3870,7 @@ static struct cpuset *nearest_hardwall_ancestor(struct cpuset *cs)
  *	GFP_KERNEL   - any node in enclosing hardwalled cpuset ok
  *	GFP_USER     - only nodes in current tasks mems allowed ok.
  */
-bool __cpuset_node_allowed(int node, gfp_t gfp_mask)
+bool cpuset_node_allowed(int node, gfp_t gfp_mask)
 {
 	struct cpuset *cs;		/* current cpuset ancestors */
 	bool allowed;			/* is allocation in zone z allowed? */
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3bb3484563ed..0d170ae590d8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4175,7 +4175,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
 retry:
 	/*
 	 * Scan zonelist, looking for a zone with enough free.
-	 * See also __cpuset_node_allowed() comment in kernel/cgroup/cpuset.c.
+	 * See also cpuset_node_allowed() comment in kernel/cgroup/cpuset.c.
 	 */
 	no_fallback = alloc_flags & ALLOC_NOFRAGMENT;
 	z = ac->preferred_zoneref;
@@ -4843,7 +4843,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
 			alloc_flags |= ALLOC_HARDER;
 		/*
 		 * Ignore cpuset mems for GFP_ATOMIC rather than fail, see the
-		 * comment for __cpuset_node_allowed().
+		 * comment for cpuset_node_allowed().
 		 */
 		alloc_flags &= ~ALLOC_CPUSET;
 	} else if (unlikely(rt_task(current)) && in_task())
-- 
2.25.1

Re: [PATCH] cpuset: Clean up cpuset_node_allowed

[Tejun Heo]

On Tue, Feb 28, 2023 at 08:35:37AM +0000, Haifeng Xu wrote:
> Commit 002f290627c2 ("cpuset: use static key better and convert to new API")
> has used __cpuset_node_allowed() instead of cpuset_node_allowed() to check
> whether we can allocate on a memory node. Now this function isn't used by
> anyone, so we can do the follow things to clean up it.
> 
> 1. remove unused codes
> 2. rename __cpuset_node_allowed() to cpuset_node_allowed()
> 3. update comments in mm/page_alloc.c
> 
> Suggested-by: Waiman Long <longman@redhat.com>
> Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>

Waiman, can you review and ack?

Thanks.

-- 
tejun

Re: [PATCH] cpuset: Clean up cpuset_node_allowed

[Waiman Long]

On 2/28/23 03:35, Haifeng Xu wrote:
> Commit 002f290627c2 ("cpuset: use static key better and convert to new API")
> has used __cpuset_node_allowed() instead of cpuset_node_allowed() to check
> whether we can allocate on a memory node. Now this function isn't used by
> anyone, so we can do the follow things to clean up it.
>
> 1. remove unused codes
> 2. rename __cpuset_node_allowed() to cpuset_node_allowed()
> 3. update comments in mm/page_alloc.c
>
> Suggested-by: Waiman Long <longman@redhat.com>
> Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
> ---
>   include/linux/cpuset.h | 16 ++--------------
>   kernel/cgroup/cpuset.c |  4 ++--
>   mm/page_alloc.c        |  4 ++--
>   3 files changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
> index d58e0476ee8e..980b76a1237e 100644
> --- a/include/linux/cpuset.h
> +++ b/include/linux/cpuset.h
> @@ -80,18 +80,11 @@ extern nodemask_t cpuset_mems_allowed(struct task_struct *p);
>   void cpuset_init_current_mems_allowed(void);
>   int cpuset_nodemask_valid_mems_allowed(nodemask_t *nodemask);
>   
> -extern bool __cpuset_node_allowed(int node, gfp_t gfp_mask);
> -
> -static inline bool cpuset_node_allowed(int node, gfp_t gfp_mask)
> -{
> -	if (cpusets_enabled())
> -		return __cpuset_node_allowed(node, gfp_mask);
> -	return true;
> -}
> +extern bool cpuset_node_allowed(int node, gfp_t gfp_mask);
>   
>   static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
>   {
> -	return __cpuset_node_allowed(zone_to_nid(z), gfp_mask);
> +	return cpuset_node_allowed(zone_to_nid(z), gfp_mask);
>   }
>   
>   static inline bool cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
> @@ -223,11 +216,6 @@ static inline int cpuset_nodemask_valid_mems_allowed(nodemask_t *nodemask)
>   	return 1;
>   }
>   
> -static inline bool cpuset_node_allowed(int node, gfp_t gfp_mask)
> -{
> -	return true;
> -}
> -

Sorry for the late reply as I apparently drop the ball.

You need to keep the above cpuset_node_allowed() inline function or you 
will get compilation error when compiling with a config without 
CONFIG_CPUSETS. Other than that, the other changes look good.

Cheers,
Longman

Re: [PATCH] cpuset: Clean up cpuset_node_allowed

[Waiman Long]

On 3/17/23 21:35, Waiman Long wrote:
> On 2/28/23 03:35, Haifeng Xu wrote:
>> Commit 002f290627c2 ("cpuset: use static key better and convert to 
>> new API")
>> has used __cpuset_node_allowed() instead of cpuset_node_allowed() to 
>> check
>> whether we can allocate on a memory node. Now this function isn't 
>> used by
>> anyone, so we can do the follow things to clean up it.
>>
>> 1. remove unused codes
>> 2. rename __cpuset_node_allowed() to cpuset_node_allowed()
>> 3. update comments in mm/page_alloc.c
>>
>> Suggested-by: Waiman Long <longman@redhat.com>
>> Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
>> ---
>>   include/linux/cpuset.h | 16 ++--------------
>>   kernel/cgroup/cpuset.c |  4 ++--
>>   mm/page_alloc.c        |  4 ++--
>>   3 files changed, 6 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
>> index d58e0476ee8e..980b76a1237e 100644
>> --- a/include/linux/cpuset.h
>> +++ b/include/linux/cpuset.h
>> @@ -80,18 +80,11 @@ extern nodemask_t cpuset_mems_allowed(struct 
>> task_struct *p);
>>   void cpuset_init_current_mems_allowed(void);
>>   int cpuset_nodemask_valid_mems_allowed(nodemask_t *nodemask);
>>   -extern bool __cpuset_node_allowed(int node, gfp_t gfp_mask);
>> -
>> -static inline bool cpuset_node_allowed(int node, gfp_t gfp_mask)
>> -{
>> -    if (cpusets_enabled())
>> -        return __cpuset_node_allowed(node, gfp_mask);
>> -    return true;
>> -}
>> +extern bool cpuset_node_allowed(int node, gfp_t gfp_mask);
>>     static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t 
>> gfp_mask)
>>   {
>> -    return __cpuset_node_allowed(zone_to_nid(z), gfp_mask);
>> +    return cpuset_node_allowed(zone_to_nid(z), gfp_mask);
>>   }
>>     static inline bool cpuset_zone_allowed(struct zone *z, gfp_t 
>> gfp_mask)
>> @@ -223,11 +216,6 @@ static inline int 
>> cpuset_nodemask_valid_mems_allowed(nodemask_t *nodemask)
>>       return 1;
>>   }
>>   -static inline bool cpuset_node_allowed(int node, gfp_t gfp_mask)
>> -{
>> -    return true;
>> -}
>> -
>
> Sorry for the late reply as I apparently drop the ball.
>
> You need to keep the above cpuset_node_allowed() inline function or 
> you will get compilation error when compiling with a config without 
> CONFIG_CPUSETS. Other than that, the other changes look good.
>
It turns out that cpuset_node_allowed() isn't used anywhere except in 
cpuset.h. So it should be OK to remove the alternate 
cpuset_node_allowed() function.

Acked-by: Waiman Long <longman@redhat.com>

Re: [PATCH] cpuset: Clean up cpuset_node_allowed

[Tejun Heo]

On Sat, Mar 18, 2023 at 05:30:46PM -0400, Waiman Long wrote:
> On 3/17/23 21:35, Waiman Long wrote:
> > On 2/28/23 03:35, Haifeng Xu wrote:
> > > Commit 002f290627c2 ("cpuset: use static key better and convert to
> > > new API")
> > > has used __cpuset_node_allowed() instead of cpuset_node_allowed() to
> > > check
> > > whether we can allocate on a memory node. Now this function isn't
> > > used by
> > > anyone, so we can do the follow things to clean up it.
> > > 
> > > 1. remove unused codes
> > > 2. rename __cpuset_node_allowed() to cpuset_node_allowed()
> > > 3. update comments in mm/page_alloc.c
> > > 
> > > Suggested-by: Waiman Long <longman@redhat.com>
> > > Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
...
> It turns out that cpuset_node_allowed() isn't used anywhere except in
> cpuset.h. So it should be OK to remove the alternate cpuset_node_allowed()
> function.
> 
> Acked-by: Waiman Long <longman@redhat.com>

Applied to cgroup/for-6.4.

Thanks.

-- 
tejun