Re: [PATCH 1/3] ceph: move to a dedicated slabcache for ceph_cap_snap

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2022-02-18 at 18:11 +0000, Luís Henriques wrote:
> xiubli@redhat.com writes:
> 
> > From: Xiubo Li <xiubli@redhat.com>
> > 
> > There could be huge number of capsnap queued in a short time, on
> > x86_64 it's 248 bytes, which will be rounded up to 256 bytes by
> > kzalloc. Move this to a dedicated slabcache to save 8 bytes for
> > each.
> > 
> > For the kmalloc-256 slab cache, the actual size will be 512 bytes:
> > kmalloc-256        21797  74656    512   32    4 : tunables, etc
> > 
> > For a dedicated slab cache the real size is 312 bytes:
> > ceph_cap_snap          0      0    312   52    4 : tunables, etc
> > 
> > So actually we can save 200 bytes for each.
> > 
> > Signed-off-by: Xiubo Li <xiubli@redhat.com>
> > ---
> >  fs/ceph/snap.c               | 5 +++--
> >  fs/ceph/super.c              | 7 +++++++
> >  fs/ceph/super.h              | 2 +-
> >  include/linux/ceph/libceph.h | 1 +
> >  4 files changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
> > index b41e6724c591..c787775eaf2a 100644
> > --- a/fs/ceph/snap.c
> > +++ b/fs/ceph/snap.c
> > @@ -482,7 +482,7 @@ static void ceph_queue_cap_snap(struct ceph_inode_info *ci)
> >  	struct ceph_buffer *old_blob = NULL;
> >  	int used, dirty;
> >  
> > -	capsnap = kzalloc(sizeof(*capsnap), GFP_NOFS);
> > +	capsnap = kmem_cache_alloc(ceph_cap_snap_cachep, GFP_NOFS);
> 
> Unfortunately, this is causing issues in my testing.  Looks like there are
> several fields that are assumed to be initialised to zero.  I've seen two
> BUGs so far, in functions ceph_try_drop_cap_snap (capsnap->cap_flush.tid > 0)
> and __ceph_finish_cap_snap (capsnap->writing).
> 
> I guess you'll have to either zero out all that memory, or manually
> initialise the fields (not sure which ones really require that).


Good catch. That memory is expected to be zeroed. I switched it to use
kmem_cache_zalloc in testing branch, which should fix this. Please let
me know if it doesn't.

-- 
Jeff Layton <jlayton@kernel.org>