[1/3] mm, slab: deprecate SLAB_MEM_SPREAD flag

Message ID 20240220-slab-cleanup-flags-v1-1-e657e373944a@suse.cz
State New
Headers
Series cleanup of SLAB_ flags |

Commit Message

Vlastimil Babka Feb. 20, 2024, 4:58 p.m. UTC
  The SLAB_MEM_SPREAD flag used to be implemented in SLAB, which was
removed.  SLUB instead relies on the page allocator's NUMA policies.
Change the flag's value to 0 to free up the value it had, and mark it
for full removal once all users are gone.

Reported-by: Steven Rostedt <rostedt@goodmis.org>
Closes: https://lore.kernel.org/all/20240131172027.10f64405@gandalf.local.home/
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 include/linux/slab.h | 5 +++--
 mm/slab.h            | 1 -
 2 files changed, 3 insertions(+), 3 deletions(-)
  

Comments

Xiongwei Song Feb. 21, 2024, 2:17 a.m. UTC | #1
> The SLAB_MEM_SPREAD flag used to be implemented in SLAB, which was
> removed.  SLUB instead relies on the page allocator's NUMA policies.
> Change the flag's value to 0 to free up the value it had, and mark it
> for full removal once all users are gone.
> 
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Closes: https://lore.kernel.org/all/20240131172027.10f64405@gandalf.local.home/
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>

Ran a rough test with build and bootup, feel free to add

Tested-by: Xiongwei Song <xiongwei.song@windriver.com>
Reviewed-by: Xiongwei Song <xiongwei.song@windriver.com>

> ---
>  include/linux/slab.h | 5 +++--
>  mm/slab.h            | 1 -
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index b5f5ee8308d0..6252f44115c2 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -96,8 +96,6 @@
>   */
>  /* Defer freeing slabs to RCU */
>  #define SLAB_TYPESAFE_BY_RCU   ((slab_flags_t __force)0x00080000U)
> -/* Spread some memory over cpuset */
> -#define SLAB_MEM_SPREAD                ((slab_flags_t __force)0x00100000U)
>  /* Trace allocations and frees */
>  #define SLAB_TRACE             ((slab_flags_t __force)0x00200000U)
> 
> @@ -164,6 +162,9 @@
>  #endif
>  #define SLAB_TEMPORARY         SLAB_RECLAIM_ACCOUNT    /* Objects are short-lived */
> 
> +/* Obsolete unused flag, to be removed */
> +#define SLAB_MEM_SPREAD                0
> +
>  /*
>   * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
>   *
> diff --git a/mm/slab.h b/mm/slab.h
> index 54deeb0428c6..f4534eefb35d 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -469,7 +469,6 @@ static inline bool is_kmalloc_cache(struct kmem_cache *s)
>                               SLAB_STORE_USER | \
>                               SLAB_TRACE | \
>                               SLAB_CONSISTENCY_CHECKS | \
> -                             SLAB_MEM_SPREAD | \
>                               SLAB_NOLEAKTRACE | \
>                               SLAB_RECLAIM_ACCOUNT | \
>                               SLAB_TEMPORARY | \
> 
> --
> 2.43.1
  
Chengming Zhou Feb. 21, 2024, 7:11 a.m. UTC | #2
On 2024/2/21 00:58, Vlastimil Babka wrote:
> The SLAB_MEM_SPREAD flag used to be implemented in SLAB, which was
> removed.  SLUB instead relies on the page allocator's NUMA policies.
> Change the flag's value to 0 to free up the value it had, and mark it
> for full removal once all users are gone.
> 
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Closes: https://lore.kernel.org/all/20240131172027.10f64405@gandalf.local.home/
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>

Reviewed-by: Chengming Zhou <chengming.zhou@linux.dev>

Thanks!

> ---
>  include/linux/slab.h | 5 +++--
>  mm/slab.h            | 1 -
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index b5f5ee8308d0..6252f44115c2 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -96,8 +96,6 @@
>   */
>  /* Defer freeing slabs to RCU */
>  #define SLAB_TYPESAFE_BY_RCU	((slab_flags_t __force)0x00080000U)
> -/* Spread some memory over cpuset */
> -#define SLAB_MEM_SPREAD		((slab_flags_t __force)0x00100000U)
>  /* Trace allocations and frees */
>  #define SLAB_TRACE		((slab_flags_t __force)0x00200000U)
>  
> @@ -164,6 +162,9 @@
>  #endif
>  #define SLAB_TEMPORARY		SLAB_RECLAIM_ACCOUNT	/* Objects are short-lived */
>  
> +/* Obsolete unused flag, to be removed */
> +#define SLAB_MEM_SPREAD		0
> +
>  /*
>   * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
>   *
> diff --git a/mm/slab.h b/mm/slab.h
> index 54deeb0428c6..f4534eefb35d 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -469,7 +469,6 @@ static inline bool is_kmalloc_cache(struct kmem_cache *s)
>  			      SLAB_STORE_USER | \
>  			      SLAB_TRACE | \
>  			      SLAB_CONSISTENCY_CHECKS | \
> -			      SLAB_MEM_SPREAD | \
>  			      SLAB_NOLEAKTRACE | \
>  			      SLAB_RECLAIM_ACCOUNT | \
>  			      SLAB_TEMPORARY | \
>
  
Roman Gushchin Feb. 21, 2024, 6:30 p.m. UTC | #3
On Tue, Feb 20, 2024 at 05:58:25PM +0100, Vlastimil Babka wrote:
0;95;0c> The SLAB_MEM_SPREAD flag used to be implemented in SLAB, which was
> removed.  SLUB instead relies on the page allocator's NUMA policies.
> Change the flag's value to 0 to free up the value it had, and mark it
> for full removal once all users are gone.
> 
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Closes: https://lore.kernel.org/all/20240131172027.10f64405@gandalf.local.home/
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>

Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>

Do you plan to follow up with a patch series removing all usages?

Thanks!
  
Xiongwei Song Feb. 22, 2024, 1:10 a.m. UTC | #4
Hi Vlastimil,

> On Tue, Feb 20, 2024 at 05:58:25PM +0100, Vlastimil Babka wrote:
> 0;95;0c> The SLAB_MEM_SPREAD flag used to be implemented in SLAB, which was
> > removed.  SLUB instead relies on the page allocator's NUMA policies.
> > Change the flag's value to 0 to free up the value it had, and mark it
> > for full removal once all users are gone.
> >
> > Reported-by: Steven Rostedt <rostedt@goodmis.org>
> > Closes: https://lore.kernel.org/all/20240131172027.10f64405@gandalf.local.home/
> > Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> 
> Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>
> 
> Do you plan to follow up with a patch series removing all usages?

If you are not available with it, I can do.

Regards,
Xiongwei
  
Chengming Zhou Feb. 22, 2024, 2:32 a.m. UTC | #5
On 2024/2/22 09:10, Song, Xiongwei wrote:
> Hi Vlastimil,
> 
>> On Tue, Feb 20, 2024 at 05:58:25PM +0100, Vlastimil Babka wrote:
>> 0;95;0c> The SLAB_MEM_SPREAD flag used to be implemented in SLAB, which was
>>> removed.  SLUB instead relies on the page allocator's NUMA policies.
>>> Change the flag's value to 0 to free up the value it had, and mark it
>>> for full removal once all users are gone.
>>>
>>> Reported-by: Steven Rostedt <rostedt@goodmis.org>
>>> Closes: https://lore.kernel.org/all/20240131172027.10f64405@gandalf.local.home/
>>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>>
>> Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>
>>
>> Do you plan to follow up with a patch series removing all usages?
> 
> If you are not available with it, I can do.

Actually, I have done it yesterday. Sorry, I just forgot this task. :)

I plan to send out it after this series merged in the slab branch. And
I'm wondering is it better to put all diffs in one huge patch or split
every diff to each patch?

Thanks!
  
Xiongwei Song Feb. 22, 2024, 3:13 a.m. UTC | #6
> On 2024/2/22 09:10, Song, Xiongwei wrote:
> > Hi Vlastimil,
> >
> >> On Tue, Feb 20, 2024 at 05:58:25PM +0100, Vlastimil Babka wrote:
> >> 0;95;0c> The SLAB_MEM_SPREAD flag used to be implemented in SLAB, which was
> >>> removed.  SLUB instead relies on the page allocator's NUMA policies.
> >>> Change the flag's value to 0 to free up the value it had, and mark it
> >>> for full removal once all users are gone.
> >>>
> >>> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> >>> Closes: https://lore.kernel.org/all/20240131172027.10f64405@gandalf.local.home/
> >>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> >>
> >> Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>
> >>
> >> Do you plan to follow up with a patch series removing all usages?
> >
> > If you are not available with it, I can do.
> 
> Actually, I have done it yesterday. Sorry, I just forgot this task. :)

Ok, that's fine.

I remember you said you wanted to do it. But it's been for a long time.
I thinks that's why Vlastimil sent the series out. 

You could've said what you've done or your any update when you reviewed
this series yesterday, which wouldn't make others confused. So keeping 
update would be better.

Thanks.

> 
> I plan to send out it after this series merged in the slab branch. And
> I'm wondering is it better to put all diffs in one huge patch or split
> every diff to each patch?
> 
> Thanks!
  
Vlastimil Babka Feb. 23, 2024, 4:41 p.m. UTC | #7
On 2/22/24 03:32, Chengming Zhou wrote:
> On 2024/2/22 09:10, Song, Xiongwei wrote:
>> Hi Vlastimil,
>> 
>>> On Tue, Feb 20, 2024 at 05:58:25PM +0100, Vlastimil Babka wrote:
>>> 0;95;0c> The SLAB_MEM_SPREAD flag used to be implemented in SLAB, which was
>>>> removed.  SLUB instead relies on the page allocator's NUMA policies.
>>>> Change the flag's value to 0 to free up the value it had, and mark it
>>>> for full removal once all users are gone.
>>>>
>>>> Reported-by: Steven Rostedt <rostedt@goodmis.org>
>>>> Closes: https://lore.kernel.org/all/20240131172027.10f64405@gandalf.local.home/
>>>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>>>
>>> Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>
>>>
>>> Do you plan to follow up with a patch series removing all usages?
>> 
>> If you are not available with it, I can do.
> 
> Actually, I have done it yesterday. Sorry, I just forgot this task. :)
> 
> I plan to send out it after this series merged in the slab branch. And
> I'm wondering is it better to put all diffs in one huge patch or split
> every diff to each patch?

I'd suggest you do a patch per subsystem (mostly different filesystems) and
send them out to respective maintainers to pick in their trees. I've talked
to David from btrfs and he suggested this way.

You don't need to wait for this series to be merged. The flag is already a
no-op as of 6.8-rc1. Also I'd suggest sending the patches individually. In a
series they wouldn't depend on each other anyway, and you would either have
to Cc maintainers separately per patch of the series, or everyone on
everything, and there would always be somebody who would prefer the other
way that you pick.

> Thanks!
  
Chengming Zhou Feb. 24, 2024, 9:32 a.m. UTC | #8
On 2024/2/24 00:41, Vlastimil Babka wrote:
> On 2/22/24 03:32, Chengming Zhou wrote:
>> On 2024/2/22 09:10, Song, Xiongwei wrote:
>>> Hi Vlastimil,
>>>
>>>> On Tue, Feb 20, 2024 at 05:58:25PM +0100, Vlastimil Babka wrote:
>>>> 0;95;0c> The SLAB_MEM_SPREAD flag used to be implemented in SLAB, which was
>>>>> removed.  SLUB instead relies on the page allocator's NUMA policies.
>>>>> Change the flag's value to 0 to free up the value it had, and mark it
>>>>> for full removal once all users are gone.
>>>>>
>>>>> Reported-by: Steven Rostedt <rostedt@goodmis.org>
>>>>> Closes: https://lore.kernel.org/all/20240131172027.10f64405@gandalf.local.home/
>>>>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>>>>
>>>> Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>
>>>>
>>>> Do you plan to follow up with a patch series removing all usages?
>>>
>>> If you are not available with it, I can do.
>>
>> Actually, I have done it yesterday. Sorry, I just forgot this task. :)
>>
>> I plan to send out it after this series merged in the slab branch. And
>> I'm wondering is it better to put all diffs in one huge patch or split
>> every diff to each patch?
> 
> I'd suggest you do a patch per subsystem (mostly different filesystems) and
> send them out to respective maintainers to pick in their trees. I've talked
> to David from btrfs and he suggested this way.

Ok, will send out individually.

> 
> You don't need to wait for this series to be merged. The flag is already a
> no-op as of 6.8-rc1. Also I'd suggest sending the patches individually. In a
> series they wouldn't depend on each other anyway, and you would either have
> to Cc maintainers separately per patch of the series, or everyone on
> everything, and there would always be somebody who would prefer the other
> way that you pick.

Right, thanks for your instructions!

> 
>> Thanks!
>
  

Patch

diff --git a/include/linux/slab.h b/include/linux/slab.h
index b5f5ee8308d0..6252f44115c2 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -96,8 +96,6 @@ 
  */
 /* Defer freeing slabs to RCU */
 #define SLAB_TYPESAFE_BY_RCU	((slab_flags_t __force)0x00080000U)
-/* Spread some memory over cpuset */
-#define SLAB_MEM_SPREAD		((slab_flags_t __force)0x00100000U)
 /* Trace allocations and frees */
 #define SLAB_TRACE		((slab_flags_t __force)0x00200000U)
 
@@ -164,6 +162,9 @@ 
 #endif
 #define SLAB_TEMPORARY		SLAB_RECLAIM_ACCOUNT	/* Objects are short-lived */
 
+/* Obsolete unused flag, to be removed */
+#define SLAB_MEM_SPREAD		0
+
 /*
  * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
  *
diff --git a/mm/slab.h b/mm/slab.h
index 54deeb0428c6..f4534eefb35d 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -469,7 +469,6 @@  static inline bool is_kmalloc_cache(struct kmem_cache *s)
 			      SLAB_STORE_USER | \
 			      SLAB_TRACE | \
 			      SLAB_CONSISTENCY_CHECKS | \
-			      SLAB_MEM_SPREAD | \
 			      SLAB_NOLEAKTRACE | \
 			      SLAB_RECLAIM_ACCOUNT | \
 			      SLAB_TEMPORARY | \