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
