All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] cpuset: Remove unused cpuset_node_allowed
@ 2023-02-27  8:07 ` Haifeng Xu
  0 siblings, 0 replies; 20+ messages in thread
From: Haifeng Xu @ 2023-02-27  8:07 UTC (permalink / raw)
  To: longman; +Cc: lizefan.x, cgroups, linux-kernel, 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 remove it safely.

Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
---
 include/linux/cpuset.h | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
index d58e0476ee8e..7fad5afe3bba 100644
--- a/include/linux/cpuset.h
+++ b/include/linux/cpuset.h
@@ -82,13 +82,6 @@ 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;
-}
-
 static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
 {
 	return __cpuset_node_allowed(zone_to_nid(z), 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;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH] cpuset: Remove unused cpuset_node_allowed
@ 2023-02-27  8:07 ` Haifeng Xu
  0 siblings, 0 replies; 20+ messages in thread
From: Haifeng Xu @ 2023-02-27  8:07 UTC (permalink / raw)
  To: longman-H+wXaHxf7aLQT0dZR+AlfA
  Cc: lizefan.x-EC8Uxl6Npydl57MIdRCFDg, cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, 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 remove it safely.

Signed-off-by: Haifeng Xu <haifeng.xu-LL2PKPoSiP3QT0dZR+AlfA@public.gmane.org>
---
 include/linux/cpuset.h | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
index d58e0476ee8e..7fad5afe3bba 100644
--- a/include/linux/cpuset.h
+++ b/include/linux/cpuset.h
@@ -82,13 +82,6 @@ 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;
-}
-
 static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
 {
 	return __cpuset_node_allowed(zone_to_nid(z), 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;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Remove unused cpuset_node_allowed
@ 2023-02-27 14:56   ` Waiman Long
  0 siblings, 0 replies; 20+ messages in thread
From: Waiman Long @ 2023-02-27 14:56 UTC (permalink / raw)
  To: Haifeng Xu; +Cc: lizefan.x, cgroups, linux-kernel

On 2/27/23 03:07, 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 remove it safely.
>
> Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
> ---
>   include/linux/cpuset.h | 12 ------------
>   1 file changed, 12 deletions(-)
>
> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
> index d58e0476ee8e..7fad5afe3bba 100644
> --- a/include/linux/cpuset.h
> +++ b/include/linux/cpuset.h
> @@ -82,13 +82,6 @@ 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;
> -}
> -
>   static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
>   {
>   	return __cpuset_node_allowed(zone_to_nid(z), 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;

The kernel convention is to add a "__" prefix to a function name if 
there is higher level helper without the "__" prefix that uses it. Since 
cpuset_node_allowed() is no longer used. We should just rename 
__cpuset_node_allowed() to cpuset_node_allowed() and get rid of the 
unused helper. A bit more code changes are needed for this, though.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Remove unused cpuset_node_allowed
@ 2023-02-27 14:56   ` Waiman Long
  0 siblings, 0 replies; 20+ messages in thread
From: Waiman Long @ 2023-02-27 14:56 UTC (permalink / raw)
  To: Haifeng Xu
  Cc: lizefan.x-EC8Uxl6Npydl57MIdRCFDg, cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 2/27/23 03:07, 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 remove it safely.
>
> Signed-off-by: Haifeng Xu <haifeng.xu-LL2PKPoSiP3QT0dZR+AlfA@public.gmane.org>
> ---
>   include/linux/cpuset.h | 12 ------------
>   1 file changed, 12 deletions(-)
>
> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
> index d58e0476ee8e..7fad5afe3bba 100644
> --- a/include/linux/cpuset.h
> +++ b/include/linux/cpuset.h
> @@ -82,13 +82,6 @@ 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;
> -}
> -
>   static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
>   {
>   	return __cpuset_node_allowed(zone_to_nid(z), 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;

The kernel convention is to add a "__" prefix to a function name if 
there is higher level helper without the "__" prefix that uses it. Since 
cpuset_node_allowed() is no longer used. We should just rename 
__cpuset_node_allowed() to cpuset_node_allowed() and get rid of the 
unused helper. A bit more code changes are needed for this, though.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Remove unused cpuset_node_allowed
@ 2023-02-28  2:40     ` Haifeng Xu
  0 siblings, 0 replies; 20+ messages in thread
From: Haifeng Xu @ 2023-02-28  2:40 UTC (permalink / raw)
  To: Waiman Long; +Cc: lizefan.x, cgroups, linux-kernel



On 2023/2/27 22:56, Waiman Long wrote:
> On 2/27/23 03:07, 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 remove it safely.
>>
>> Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
>> ---
>>   include/linux/cpuset.h | 12 ------------
>>   1 file changed, 12 deletions(-)
>>
>> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
>> index d58e0476ee8e..7fad5afe3bba 100644
>> --- a/include/linux/cpuset.h
>> +++ b/include/linux/cpuset.h
>> @@ -82,13 +82,6 @@ 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;
>> -}
>> -
>>   static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
>>   {
>>       return __cpuset_node_allowed(zone_to_nid(z), 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;
> 
> The kernel convention is to add a "__" prefix to a function name if there is higher level helper without the "__" prefix that uses it. Since cpuset_node_allowed() is no longer used. We should just rename __cpuset_node_allowed() to cpuset_node_allowed() and get rid of the unused helper. A bit more code changes are needed for this, though.
> 
> Cheers,
> Longman
> 

Maybe we can still use cpuset_node_allowed in __cpuset_zone_allowed? If so, less code need to be changed.

Thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Remove unused cpuset_node_allowed
@ 2023-02-28  2:40     ` Haifeng Xu
  0 siblings, 0 replies; 20+ messages in thread
From: Haifeng Xu @ 2023-02-28  2:40 UTC (permalink / raw)
  To: Waiman Long
  Cc: lizefan.x-EC8Uxl6Npydl57MIdRCFDg, cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA



On 2023/2/27 22:56, Waiman Long wrote:
> On 2/27/23 03:07, 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 remove it safely.
>>
>> Signed-off-by: Haifeng Xu <haifeng.xu-LL2PKPoSiP3QT0dZR+AlfA@public.gmane.org>
>> ---
>>   include/linux/cpuset.h | 12 ------------
>>   1 file changed, 12 deletions(-)
>>
>> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
>> index d58e0476ee8e..7fad5afe3bba 100644
>> --- a/include/linux/cpuset.h
>> +++ b/include/linux/cpuset.h
>> @@ -82,13 +82,6 @@ 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;
>> -}
>> -
>>   static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
>>   {
>>       return __cpuset_node_allowed(zone_to_nid(z), 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;
> 
> The kernel convention is to add a "__" prefix to a function name if there is higher level helper without the "__" prefix that uses it. Since cpuset_node_allowed() is no longer used. We should just rename __cpuset_node_allowed() to cpuset_node_allowed() and get rid of the unused helper. A bit more code changes are needed for this, though.
> 
> Cheers,
> Longman
> 

Maybe we can still use cpuset_node_allowed in __cpuset_zone_allowed? If so, less code need to be changed.

Thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Remove unused cpuset_node_allowed
@ 2023-02-28  2:52       ` Waiman Long
  0 siblings, 0 replies; 20+ messages in thread
From: Waiman Long @ 2023-02-28  2:52 UTC (permalink / raw)
  To: Haifeng Xu; +Cc: lizefan.x, cgroups, linux-kernel

On 2/27/23 21:40, Haifeng Xu wrote:
>
> On 2023/2/27 22:56, Waiman Long wrote:
>> On 2/27/23 03:07, 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 remove it safely.
>>>
>>> Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
>>> ---
>>>    include/linux/cpuset.h | 12 ------------
>>>    1 file changed, 12 deletions(-)
>>>
>>> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
>>> index d58e0476ee8e..7fad5afe3bba 100644
>>> --- a/include/linux/cpuset.h
>>> +++ b/include/linux/cpuset.h
>>> @@ -82,13 +82,6 @@ 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;
>>> -}
>>> -
>>>    static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
>>>    {
>>>        return __cpuset_node_allowed(zone_to_nid(z), 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;
>> The kernel convention is to add a "__" prefix to a function name if there is higher level helper without the "__" prefix that uses it. Since cpuset_node_allowed() is no longer used. We should just rename __cpuset_node_allowed() to cpuset_node_allowed() and get rid of the unused helper. A bit more code changes are needed for this, though.
>>
>> Cheers,
>> Longman
>>
> Maybe we can still use cpuset_node_allowed in __cpuset_zone_allowed? If so, less code need to be changed.
>
mm/page_alloc.c has 2 references to __cpuset_node_allowed() in their 
comments. These references should also be updated as well. That is why I 
said a bit more code needs to be touched.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Remove unused cpuset_node_allowed
@ 2023-02-28  2:52       ` Waiman Long
  0 siblings, 0 replies; 20+ messages in thread
From: Waiman Long @ 2023-02-28  2:52 UTC (permalink / raw)
  To: Haifeng Xu
  Cc: lizefan.x-EC8Uxl6Npydl57MIdRCFDg, cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 2/27/23 21:40, Haifeng Xu wrote:
>
> On 2023/2/27 22:56, Waiman Long wrote:
>> On 2/27/23 03:07, 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 remove it safely.
>>>
>>> Signed-off-by: Haifeng Xu <haifeng.xu-LL2PKPoSiP3QT0dZR+AlfA@public.gmane.org>
>>> ---
>>>    include/linux/cpuset.h | 12 ------------
>>>    1 file changed, 12 deletions(-)
>>>
>>> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
>>> index d58e0476ee8e..7fad5afe3bba 100644
>>> --- a/include/linux/cpuset.h
>>> +++ b/include/linux/cpuset.h
>>> @@ -82,13 +82,6 @@ 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;
>>> -}
>>> -
>>>    static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
>>>    {
>>>        return __cpuset_node_allowed(zone_to_nid(z), 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;
>> The kernel convention is to add a "__" prefix to a function name if there is higher level helper without the "__" prefix that uses it. Since cpuset_node_allowed() is no longer used. We should just rename __cpuset_node_allowed() to cpuset_node_allowed() and get rid of the unused helper. A bit more code changes are needed for this, though.
>>
>> Cheers,
>> Longman
>>
> Maybe we can still use cpuset_node_allowed in __cpuset_zone_allowed? If so, less code need to be changed.
>
mm/page_alloc.c has 2 references to __cpuset_node_allowed() in their 
comments. These references should also be updated as well. That is why I 
said a bit more code needs to be touched.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Remove unused cpuset_node_allowed
@ 2023-02-28  3:13         ` Haifeng Xu
  0 siblings, 0 replies; 20+ messages in thread
From: Haifeng Xu @ 2023-02-28  3:13 UTC (permalink / raw)
  To: Waiman Long; +Cc: lizefan.x, cgroups, linux-kernel



On 2023/2/28 10:52, Waiman Long wrote:
> On 2/27/23 21:40, Haifeng Xu wrote:
>>
>> On 2023/2/27 22:56, Waiman Long wrote:
>>> On 2/27/23 03:07, 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 remove it safely.
>>>>
>>>> Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
>>>> ---
>>>>    include/linux/cpuset.h | 12 ------------
>>>>    1 file changed, 12 deletions(-)
>>>>
>>>> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
>>>> index d58e0476ee8e..7fad5afe3bba 100644
>>>> --- a/include/linux/cpuset.h
>>>> +++ b/include/linux/cpuset.h
>>>> @@ -82,13 +82,6 @@ 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;
>>>> -}
>>>> -
>>>>    static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
>>>>    {
>>>>        return __cpuset_node_allowed(zone_to_nid(z), 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;
>>> The kernel convention is to add a "__" prefix to a function name if there is higher level helper without the "__" prefix that uses it. Since cpuset_node_allowed() is no longer used. We should just rename __cpuset_node_allowed() to cpuset_node_allowed() and get rid of the unused helper. A bit more code changes are needed for this, though.
>>>
>>> Cheers,
>>> Longman
>>>
>> Maybe we can still use cpuset_node_allowed in __cpuset_zone_allowed? If so, less code need to be changed.
>>
> mm/page_alloc.c has 2 references to __cpuset_node_allowed() in their comments. These references should also be updated as well. That is why I said a bit more code needs to be touched.
> 
> Cheers,
> Longman
> 

Ok. I mean maybe we could keep cpuset_node_allowed() and use it in __cpuset_zone_allowed(). So we don't need rename __cpuset_node_allowed() or update releated comments.

Thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Remove unused cpuset_node_allowed
@ 2023-02-28  3:13         ` Haifeng Xu
  0 siblings, 0 replies; 20+ messages in thread
From: Haifeng Xu @ 2023-02-28  3:13 UTC (permalink / raw)
  To: Waiman Long
  Cc: lizefan.x-EC8Uxl6Npydl57MIdRCFDg, cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA



On 2023/2/28 10:52, Waiman Long wrote:
> On 2/27/23 21:40, Haifeng Xu wrote:
>>
>> On 2023/2/27 22:56, Waiman Long wrote:
>>> On 2/27/23 03:07, 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 remove it safely.
>>>>
>>>> Signed-off-by: Haifeng Xu <haifeng.xu-LL2PKPoSiP3QT0dZR+AlfA@public.gmane.org>
>>>> ---
>>>>    include/linux/cpuset.h | 12 ------------
>>>>    1 file changed, 12 deletions(-)
>>>>
>>>> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
>>>> index d58e0476ee8e..7fad5afe3bba 100644
>>>> --- a/include/linux/cpuset.h
>>>> +++ b/include/linux/cpuset.h
>>>> @@ -82,13 +82,6 @@ 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;
>>>> -}
>>>> -
>>>>    static inline bool __cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)
>>>>    {
>>>>        return __cpuset_node_allowed(zone_to_nid(z), 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;
>>> The kernel convention is to add a "__" prefix to a function name if there is higher level helper without the "__" prefix that uses it. Since cpuset_node_allowed() is no longer used. We should just rename __cpuset_node_allowed() to cpuset_node_allowed() and get rid of the unused helper. A bit more code changes are needed for this, though.
>>>
>>> Cheers,
>>> Longman
>>>
>> Maybe we can still use cpuset_node_allowed in __cpuset_zone_allowed? If so, less code need to be changed.
>>
> mm/page_alloc.c has 2 references to __cpuset_node_allowed() in their comments. These references should also be updated as well. That is why I said a bit more code needs to be touched.
> 
> Cheers,
> Longman
> 

Ok. I mean maybe we could keep cpuset_node_allowed() and use it in __cpuset_zone_allowed(). So we don't need rename __cpuset_node_allowed() or update releated comments.

Thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] cpuset: Clean up cpuset_node_allowed
@ 2023-02-28  8:35     ` Haifeng Xu
  0 siblings, 0 replies; 20+ messages in thread
From: Haifeng Xu @ 2023-02-28  8:35 UTC (permalink / raw)
  To: longman
  Cc: lizefan.x, tj, hannes, akpm, cgroups, linux-mm, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH] cpuset: Clean up cpuset_node_allowed
@ 2023-02-28  8:35     ` Haifeng Xu
  0 siblings, 0 replies; 20+ messages in thread
From: Haifeng Xu @ 2023-02-28  8:35 UTC (permalink / raw)
  To: longman-H+wXaHxf7aLQT0dZR+AlfA
  Cc: lizefan.x-EC8Uxl6Npydl57MIdRCFDg, tj-DgEjT+Ai2ygdnm+yROfE0A,
	hannes-druUgvl0LCNAfugRpC6u6w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, 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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Haifeng Xu <haifeng.xu-LL2PKPoSiP3QT0dZR+AlfA@public.gmane.org>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Clean up cpuset_node_allowed
@ 2023-03-17 21:37       ` Tejun Heo
  0 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2023-03-17 21:37 UTC (permalink / raw)
  To: Haifeng Xu
  Cc: longman, lizefan.x, hannes, akpm, cgroups, linux-mm, linux-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Clean up cpuset_node_allowed
@ 2023-03-17 21:37       ` Tejun Heo
  0 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2023-03-17 21:37 UTC (permalink / raw)
  To: Haifeng Xu
  Cc: longman-H+wXaHxf7aLQT0dZR+AlfA, lizefan.x-EC8Uxl6Npydl57MIdRCFDg,
	hannes-druUgvl0LCNAfugRpC6u6w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Haifeng Xu <haifeng.xu-LL2PKPoSiP3QT0dZR+AlfA@public.gmane.org>

Waiman, can you review and ack?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Clean up cpuset_node_allowed
@ 2023-03-18  1:35       ` Waiman Long
  0 siblings, 0 replies; 20+ messages in thread
From: Waiman Long @ 2023-03-18  1:35 UTC (permalink / raw)
  To: Haifeng Xu; +Cc: lizefan.x, tj, hannes, akpm, cgroups, linux-mm, linux-kernel

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Clean up cpuset_node_allowed
@ 2023-03-18  1:35       ` Waiman Long
  0 siblings, 0 replies; 20+ messages in thread
From: Waiman Long @ 2023-03-18  1:35 UTC (permalink / raw)
  To: Haifeng Xu
  Cc: lizefan.x-EC8Uxl6Npydl57MIdRCFDg, tj-DgEjT+Ai2ygdnm+yROfE0A,
	hannes-druUgvl0LCNAfugRpC6u6w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Haifeng Xu <haifeng.xu-LL2PKPoSiP3QT0dZR+AlfA@public.gmane.org>
> ---
>   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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Clean up cpuset_node_allowed
@ 2023-03-18 21:30         ` Waiman Long
  0 siblings, 0 replies; 20+ messages in thread
From: Waiman Long @ 2023-03-18 21:30 UTC (permalink / raw)
  To: Haifeng Xu; +Cc: lizefan.x, tj, hannes, akpm, cgroups, linux-mm, linux-kernel

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>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Clean up cpuset_node_allowed
@ 2023-03-18 21:30         ` Waiman Long
  0 siblings, 0 replies; 20+ messages in thread
From: Waiman Long @ 2023-03-18 21:30 UTC (permalink / raw)
  To: Haifeng Xu
  Cc: lizefan.x-EC8Uxl6Npydl57MIdRCFDg, tj-DgEjT+Ai2ygdnm+yROfE0A,
	hannes-druUgvl0LCNAfugRpC6u6w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Haifeng Xu <haifeng.xu-LL2PKPoSiP3QT0dZR+AlfA@public.gmane.org>
>> ---
>>   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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Clean up cpuset_node_allowed
@ 2023-03-24  2:03           ` Tejun Heo
  0 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2023-03-24  2:03 UTC (permalink / raw)
  To: Waiman Long
  Cc: Haifeng Xu, lizefan.x, hannes, akpm, cgroups, linux-mm, linux-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] cpuset: Clean up cpuset_node_allowed
@ 2023-03-24  2:03           ` Tejun Heo
  0 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2023-03-24  2:03 UTC (permalink / raw)
  To: Waiman Long
  Cc: Haifeng Xu, lizefan.x-EC8Uxl6Npydl57MIdRCFDg,
	hannes-druUgvl0LCNAfugRpC6u6w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > Signed-off-by: Haifeng Xu <haifeng.xu-LL2PKPoSiP3QT0dZR+AlfA@public.gmane.org>
...
> 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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Applied to cgroup/for-6.4.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-03-24  2:03 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-27  8:07 [PATCH] cpuset: Remove unused cpuset_node_allowed Haifeng Xu
2023-02-27  8:07 ` Haifeng Xu
2023-02-27 14:56 ` Waiman Long
2023-02-27 14:56   ` Waiman Long
2023-02-28  2:40   ` Haifeng Xu
2023-02-28  2:40     ` Haifeng Xu
2023-02-28  2:52     ` Waiman Long
2023-02-28  2:52       ` Waiman Long
2023-02-28  3:13       ` Haifeng Xu
2023-02-28  3:13         ` Haifeng Xu
2023-02-28  8:35   ` [PATCH] cpuset: Clean up cpuset_node_allowed Haifeng Xu
2023-02-28  8:35     ` Haifeng Xu
2023-03-17 21:37     ` Tejun Heo
2023-03-17 21:37       ` Tejun Heo
2023-03-18  1:35     ` Waiman Long
2023-03-18  1:35       ` Waiman Long
2023-03-18 21:30       ` Waiman Long
2023-03-18 21:30         ` Waiman Long
2023-03-24  2:03         ` Tejun Heo
2023-03-24  2:03           ` Tejun Heo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.