I was looking into moving eventfs_inode into a slab, and after cutting and
pasting the tracefs allocator:
tracefs_inode_cachep = kmem_cache_create("tracefs_inode_cache",
sizeof(struct tracefs_inode),
0, (SLAB_RECLAIM_ACCOUNT|
SLAB_MEM_SPREAD|
SLAB_ACCOUNT),
init_once);
I figured I should know what those slab flags mean. I also looked at what
others in fs use for their slabs. The above is rather common (which I
probably just copied from another file system), but I wanted to know what
they are for.
When I got to SLAB_MEM_SPREAD, I found that it's a common flag and there's
a lot of caches that just set that and nothing else.
But I couldn't find how it was used.
Then I found this commit:
16a1d968358a ("mm/slab: remove mm/slab.c and slab_def.h")
Which I think removed the only use case of SLAB_MEM_SPREAD.
$ git grep SLAB_MEM_SPREAD mm
mm/slab.h: SLAB_MEM_SPREAD | \
That's all I find in the mm directory.
Is it obsolete now? Can we delete it? Maybe there's other SLAB_* flags that
are no longer used. I don't know, I haven't audited them.
-- Steve
On Wed, Jan 31, 2024 at 2:20 PM Steven Rostedt <[email protected]> wrote:
>
> I was looking into moving eventfs_inode into a slab, and after cutting and
> pasting the tracefs allocator:
>
> tracefs_inode_cachep = kmem_cache_create("tracefs_inode_cache",
> sizeof(struct tracefs_inode),
> 0, (SLAB_RECLAIM_ACCOUNT|
> SLAB_MEM_SPREAD|
> SLAB_ACCOUNT),
> init_once);
>
> I figured I should know what those slab flags mean. I also looked at what
> others in fs use for their slabs. The above is rather common (which I
> probably just copied from another file system), but I wanted to know what
> they are for.
>
> When I got to SLAB_MEM_SPREAD, I found that it's a common flag and there's
> a lot of caches that just set that and nothing else.
>
> But I couldn't find how it was used.
>
> Then I found this commit:
>
> 16a1d968358a ("mm/slab: remove mm/slab.c and slab_def.h")
>
> Which I think removed the only use case of SLAB_MEM_SPREAD.
>
> $ git grep SLAB_MEM_SPREAD mm
> mm/slab.h: SLAB_MEM_SPREAD | \
>
> That's all I find in the mm directory.
>
> Is it obsolete now? Can we delete it? Maybe there's other SLAB_* flags that
> are no longer used. I don't know, I haven't audited them.
Perhaps cpuset_do_slab_mem_spread() as well.
On 1/31/24 23:25, Yosry Ahmed wrote:
> On Wed, Jan 31, 2024 at 2:20 PM Steven Rostedt <[email protected]> wrote:
>>
>> I was looking into moving eventfs_inode into a slab, and after cutting and
>> pasting the tracefs allocator:
>>
>> tracefs_inode_cachep = kmem_cache_create("tracefs_inode_cache",
>> sizeof(struct tracefs_inode),
>> 0, (SLAB_RECLAIM_ACCOUNT|
>> SLAB_MEM_SPREAD|
>> SLAB_ACCOUNT),
>> init_once);
>>
>> I figured I should know what those slab flags mean. I also looked at what
>> others in fs use for their slabs. The above is rather common (which I
>> probably just copied from another file system), but I wanted to know what
>> they are for.
>>
>> When I got to SLAB_MEM_SPREAD, I found that it's a common flag and there's
>> a lot of caches that just set that and nothing else.
>>
>> But I couldn't find how it was used.
>>
>> Then I found this commit:
>>
>> 16a1d968358a ("mm/slab: remove mm/slab.c and slab_def.h")
>>
>> Which I think removed the only use case of SLAB_MEM_SPREAD.
>>
>> $ git grep SLAB_MEM_SPREAD mm
>> mm/slab.h: SLAB_MEM_SPREAD | \
>>
>> That's all I find in the mm directory.
>>
>> Is it obsolete now? Can we delete it? Maybe there's other SLAB_* flags that
>> are no longer used. I don't know, I haven't audited them.
>
> Perhaps cpuset_do_slab_mem_spread() as well.
Yep, good find. Show how obscure mm/slab.c was in the end :)
CCing a few more new people who did slab changes recently, who'd like some
low hanging fruit of negative diffcount? :)
Thanks,
Vlastimil
On 2024/2/1 06:40, Vlastimil Babka wrote:
> On 1/31/24 23:25, Yosry Ahmed wrote:
>> On Wed, Jan 31, 2024 at 2:20 PM Steven Rostedt <[email protected]> wrote:
>>>
>>> I was looking into moving eventfs_inode into a slab, and after cutting and
>>> pasting the tracefs allocator:
>>>
>>> tracefs_inode_cachep = kmem_cache_create("tracefs_inode_cache",
>>> sizeof(struct tracefs_inode),
>>> 0, (SLAB_RECLAIM_ACCOUNT|
>>> SLAB_MEM_SPREAD|
>>> SLAB_ACCOUNT),
>>> init_once);
>>>
>>> I figured I should know what those slab flags mean. I also looked at what
>>> others in fs use for their slabs. The above is rather common (which I
>>> probably just copied from another file system), but I wanted to know what
>>> they are for.
>>>
>>> When I got to SLAB_MEM_SPREAD, I found that it's a common flag and there's
>>> a lot of caches that just set that and nothing else.
>>>
>>> But I couldn't find how it was used.
>>>
>>> Then I found this commit:
>>>
>>> 16a1d968358a ("mm/slab: remove mm/slab.c and slab_def.h")
>>>
>>> Which I think removed the only use case of SLAB_MEM_SPREAD.
>>>
>>> $ git grep SLAB_MEM_SPREAD mm
>>> mm/slab.h: SLAB_MEM_SPREAD | \
>>>
>>> That's all I find in the mm directory.
>>>
>>> Is it obsolete now? Can we delete it? Maybe there's other SLAB_* flags that
>>> are no longer used. I don't know, I haven't audited them.
>>
>> Perhaps cpuset_do_slab_mem_spread() as well.
>
> Yep, good find. Show how obscure mm/slab.c was in the end :)
>
> CCing a few more new people who did slab changes recently, who'd like some
> low hanging fruit of negative diffcount? :)
Thanks for CCing, I can prepare the patch to do it. IIUC, what I need to do is:
1. delete SLAB_MEM_SPREAD and all its uses.
2. cpuset_do_slab_mem_spread() is not used anymore, should we keep the interface?
Since it's the interface exported by cgroup-v1 "cpuset.memory_spread_slab".
> -----Original Message-----
> From: Chengming Zhou <[email protected]>
> Sent: Thursday, February 1, 2024 2:27 PM
> To: Vlastimil Babka <[email protected]>; Yosry Ahmed
> <[email protected]>; Steven Rostedt <[email protected]>
> Cc: LKML <[email protected]>; [email protected]; Andrew
> Morton <[email protected]>; Linus Torvalds <torvalds@linux-
> foundation.org>; Kees Cook <[email protected]>; Christoph Lameter
> <[email protected]>; David Rientjes <[email protected]>; Hyeonggon Yoo
> <[email protected]>; Song, Xiongwei <[email protected]>;
> Chengming Zhou <[email protected]>; Zheng Yejian
> <[email protected]>
> Subject: Re: Do we still need SLAB_MEM_SPREAD (and possibly others)?
>
>
> On 2024/2/1 06:40, Vlastimil Babka wrote:
> > On 1/31/24 23:25, Yosry Ahmed wrote:
> >> On Wed, Jan 31, 2024 at 2:20 PM Steven Rostedt <[email protected]>
> wrote:
> >>>
> >>> I was looking into moving eventfs_inode into a slab, and after cutting and
> >>> pasting the tracefs allocator:
> >>>
> >>> tracefs_inode_cachep = kmem_cache_create("tracefs_inode_cache",
> >>> sizeof(struct tracefs_inode),
> >>> 0, (SLAB_RECLAIM_ACCOUNT|
> >>> SLAB_MEM_SPREAD|
> >>> SLAB_ACCOUNT),
> >>> init_once);
> >>>
> >>> I figured I should know what those slab flags mean. I also looked at what
> >>> others in fs use for their slabs. The above is rather common (which I
> >>> probably just copied from another file system), but I wanted to know
> what
> >>> they are for.
> >>>
> >>> When I got to SLAB_MEM_SPREAD, I found that it's a common flag and
> there's
> >>> a lot of caches that just set that and nothing else.
> >>>
> >>> But I couldn't find how it was used.
> >>>
> >>> Then I found this commit:
> >>>
> >>> 16a1d968358a ("mm/slab: remove mm/slab.c and slab_def.h")
> >>>
> >>> Which I think removed the only use case of SLAB_MEM_SPREAD.
> >>>
> >>> $ git grep SLAB_MEM_SPREAD mm
> >>> mm/slab.h: SLAB_MEM_SPREAD | \
> >>>
> >>> That's all I find in the mm directory.
> >>>
> >>> Is it obsolete now? Can we delete it? Maybe there's other SLAB_* flags
> that
> >>> are no longer used. I don't know, I haven't audited them.
> >>
> >> Perhaps cpuset_do_slab_mem_spread() as well.
> >
> > Yep, good find. Show how obscure mm/slab.c was in the end :)
> >
> > CCing a few more new people who did slab changes recently, who'd like
> some
> > low hanging fruit of negative diffcount? :)
>
> Thanks for CCing, I can prepare the patch to do it. IIUC, what I need to do is:
>
> 1. delete SLAB_MEM_SPREAD and all its uses.
>
> 2. cpuset_do_slab_mem_spread() is not used anymore, should we keep the
> interface?
> Since it's the interface exported by cgroup-v1
> "cpuset.memory_spread_slab".
Once SLAB_MEM_SPREAD is removed, IMO, cpuset.memory_spread_slab is useless.
Regards,
Xiongwei
On 2/5/24 20:46, Song, Xiongwei wrote:
> Adding the maintainers of cpuset of cgroup.
>
>> On Sun, 4 Feb 2024, Song, Xiongwei wrote:
>>
>>> Once SLAB_MEM_SPREAD is removed, IMO, cpuset.memory_spread_slab is useless.
>> SLAB_MEM_SPREAD does not do anything anymore. SLUB relies on the
>> "spreading" via the page allocator memory policies instead of doing its
>> own like SLAB used to do.
>>
>> What does FILE_SPREAD_SLAB do? Dont see anything there either.
> The FILE_SPREAD_SLAB flag is used by cpuset.memory_spread_slab with read/write operations:
>
> In kernel/cgroup/cpuset.c,
> static struct cftype legacy_files[] = {
> ... snip ...
> {
> .name = "memory_spread_slab",
> .read_u64 = cpuset_read_u64,
> .write_u64 = cpuset_write_u64,
> .private = FILE_SPREAD_SLAB,
> },
> ... snip ...
> };
It looks like that memory_spread_slab may have effect only on the slab
allocator. With the removal of the slab allocator, memory_spread_slab is
now a no-op. However, the memory_spread_slab cgroupfs file is an
externally visible API. So we can't just remove it as it may break
existing applications. We can certainly deprecate it and advise users
not to use it.
Cheers,
Longman
On 2/5/24 22:16, Waiman Long wrote:
>
> On 2/5/24 20:46, Song, Xiongwei wrote:
>> Adding the maintainers of cpuset of cgroup.
>>
>>> On Sun, 4 Feb 2024, Song, Xiongwei wrote:
>>>
>>>> Once SLAB_MEM_SPREAD is removed, IMO, cpuset.memory_spread_slab is
>>>> useless.
>>> SLAB_MEM_SPREAD does not do anything anymore. SLUB relies on the
>>> "spreading" via the page allocator memory policies instead of doing its
>>> own like SLAB used to do.
>>>
>>> What does FILE_SPREAD_SLAB do? Dont see anything there either.
>> The FILE_SPREAD_SLAB flag is used by cpuset.memory_spread_slab with
>> read/write operations:
>>
>> In kernel/cgroup/cpuset.c,
>> static struct cftype legacy_files[] = {
>> ... snip ...
>> {
>> .name = "memory_spread_slab",
>> .read_u64 = cpuset_read_u64,
>> .write_u64 = cpuset_write_u64,
>> .private = FILE_SPREAD_SLAB,
>> },
>> ... snip ...
>> };
>
> It looks like that memory_spread_slab may have effect only on the slab
> allocator. With the removal of the slab allocator, memory_spread_slab
> is now a no-op. However, the memory_spread_slab cgroupfs file is an
> externally visible API. So we can't just remove it as it may break
> existing applications. We can certainly deprecate it and advise users
> not to use it.
BTW, cpuset doesn't use SLAB_MEM_SPREAD directly. Instead it set the
task's PFA_SPREAD_SLAB and let other subsystems test it to act
appropriately. Other than cpuset, the latest upstream kernel doesn't
check or use this flag at all.
Cheers,
Longman
On 2024/2/6 11:20, Waiman Long wrote:
>
> On 2/5/24 22:16, Waiman Long wrote:
>>
>> On 2/5/24 20:46, Song, Xiongwei wrote:
>>> Adding the maintainers of cpuset of cgroup.
>>>
>>>> On Sun, 4 Feb 2024, Song, Xiongwei wrote:
>>>>
>>>>> Once SLAB_MEM_SPREAD is removed, IMO, cpuset.memory_spread_slab is useless.
>>>> SLAB_MEM_SPREAD does not do anything anymore. SLUB relies on the
>>>> "spreading" via the page allocator memory policies instead of doing its
>>>> own like SLAB used to do.
>>>>
>>>> What does FILE_SPREAD_SLAB do? Dont see anything there either.
>>> The FILE_SPREAD_SLAB flag is used by cpuset.memory_spread_slab with read/write operations:
>>>
>>> In kernel/cgroup/cpuset.c,
>>> static struct cftype legacy_files[] = {
>>> ... snip ...
>>> {
>>> .name = "memory_spread_slab",
>>> .read_u64 = cpuset_read_u64,
>>> .write_u64 = cpuset_write_u64,
>>> .private = FILE_SPREAD_SLAB,
>>> },
>>> ... snip ...
>>> };
>>
>> It looks like that memory_spread_slab may have effect only on the slab allocator. With the removal of the slab allocator, memory_spread_slab is now a no-op. However, the memory_spread_slab cgroupfs file is an externally visible API. So we can't just remove it as it may break existing applications. We can certainly deprecate it and advise users not to use it.
>
> BTW, cpuset doesn't use SLAB_MEM_SPREAD directly. Instead it set the task's PFA_SPREAD_SLAB and let other subsystems test it to act appropriately. Other than cpuset, the latest upstream kernel doesn't check or use this flag at all.
>
Ok, get it. So cpuset_do_slab_mem_spread() can be removed, but
this cpuset file interface and PFA_SPREAD_SLAB will be keeped.
Thanks.
On 2/5/24 22:25, Chengming Zhou wrote:
> On 2024/2/6 11:20, Waiman Long wrote:
>> On 2/5/24 22:16, Waiman Long wrote:
>>> On 2/5/24 20:46, Song, Xiongwei wrote:
>>>> Adding the maintainers of cpuset of cgroup.
>>>>
>>>>> On Sun, 4 Feb 2024, Song, Xiongwei wrote:
>>>>>
>>>>>> Once SLAB_MEM_SPREAD is removed, IMO, cpuset.memory_spread_slab is useless.
>>>>> SLAB_MEM_SPREAD does not do anything anymore. SLUB relies on the
>>>>> "spreading" via the page allocator memory policies instead of doing its
>>>>> own like SLAB used to do.
>>>>>
>>>>> What does FILE_SPREAD_SLAB do? Dont see anything there either.
>>>> The FILE_SPREAD_SLAB flag is used by cpuset.memory_spread_slab with read/write operations:
>>>>
>>>> In kernel/cgroup/cpuset.c,
>>>> static struct cftype legacy_files[] = {
>>>> ... snip ...
>>>> {
>>>> .name = "memory_spread_slab",
>>>> .read_u64 = cpuset_read_u64,
>>>> .write_u64 = cpuset_write_u64,
>>>> .private = FILE_SPREAD_SLAB,
>>>> },
>>>> ... snip ...
>>>> };
>>> It looks like that memory_spread_slab may have effect only on the slab allocator. With the removal of the slab allocator, memory_spread_slab is now a no-op. However, the memory_spread_slab cgroupfs file is an externally visible API. So we can't just remove it as it may break existing applications. We can certainly deprecate it and advise users not to use it.
>> BTW, cpuset doesn't use SLAB_MEM_SPREAD directly. Instead it set the task's PFA_SPREAD_SLAB and let other subsystems test it to act appropriately. Other than cpuset, the latest upstream kernel doesn't check or use this flag at all.
>>
> Ok, get it. So cpuset_do_slab_mem_spread() can be removed, but
> this cpuset file interface and PFA_SPREAD_SLAB will be keeped.
Yes, for now.
Cheers,
Longman