Re: slab warning: kmem_cache of name 'dm_bufio_buffer' already exists

[Mikulas Patocka <mpatocka@redhat.com>]

On Wed, 6 Nov 2024, Vlastimil Babka wrote:

> On 11/6/24 13:05, Mikulas Patocka wrote:
> > 
> > 
> > On Wed, 6 Nov 2024, Vlastimil Babka wrote:
> > 
> >> On 11/6/24 12:19, Mikulas Patocka wrote:
> >> > Hi
> >> 
> >> Hi,
> >> 
> >> > The commit 4c39529663b93165953ecf9b1a9ea817358dcd06 ("slab: Warn on 
> >> > duplicate cache names when DEBUG_VM=y") is causing large number of 
> >> > warnings about "dm_bufio_buffer", "dm_bufio_buffer-%u" (and other) device 
> >> > mapper caches.
> >> 
> >> Hmm wonder why nobody run into this before. We thought the code that would
> >> cause the warning would be all fixed before introducing it, but we missed
> >> some, sorry.
> >> 
> >> > I'd like to ask - how to properly fix it?
> >> > 
> >> > We create a "dm_bufio_buffer" or "dm_bufio_buffer-%u" cache with every dm 
> >> > bufio client. It used to work (and the duplicate caches are merged), but 
> >> 
> >> Note the merging can be disabled so then it's really several caches with
> >> exactly same name in /proc/slabinfo and inability to create their
> >> sysfs/debugfs directories.
> > 
> > Would it be sensible to allow merging caches with the same name and same 
> > attributes and only warn if there are caches with the same name and 
> > different attributes?
> 
> We might consider that.

That would be good - so that users don't have to write their own slab 
cache merge logic.

> BTW, what benefits do you get from creating own kmem caches instead of using
> kmalloc()? If it's just alignment, if you round up the intended size to
> power of two, there's implicit kmalloc alignment guarantee.

See the function xfs_buf_alloc_kmem - it allocates a buffer using kmalloc, 
tests if the buffer crosses a page boundary, and if it does, the code 
falls back to xfs_buf_alloc_pages.

Do you think that it can be simplified to just allocate a buffer and NOT 
check for page crossing?

> AFAICS there's some alignment for c->slab_cache in 
> dm_bufio_client_create()

There are two slab caches - one for the dm_buffer structure and one for 
the buffer data (if the buffer size is less than a page).

> In case the allocations have odd sizes without any such alignment
> (the case of c->slab_buffer?) separate size-specific caches can result in
> better packing, but that should only matter if you expect many/long-lived
> objects to be allocated.

The cache for the dm_buffer structure is there so that it utilizes memory 
better. Yes - there may be a lot of long-lived dm_buffers in memory.

Mikulas