Messages in this thread |  | | Date | Mon, 4 Jan 2021 09:54:21 +0100 | From | Christoph Hellwig <> | Subject | Re: [PATCH 1/6] block: manage bio slab cache by xarray |
| |
On Wed, Dec 30, 2020 at 08:32:50AM +0800, Ming Lei wrote: > Managing bio slab cache via xarray by using slab cache size as xarray index, and
Overly long line in the commit log above.
> +static struct bio_slab *create_bio_slab(unsigned int size) > +{ > + struct bio_slab *bslab = kzalloc(sizeof(*bslab), GFP_KERNEL); > + if (!bslab) > + return NULL;
Missing whitespace after the variable declaration.
> + > + snprintf(bslab->name, sizeof(bslab->name), "bio-%d", size); > + bslab->slab = kmem_cache_create(bslab->name, size, > + ARCH_KMALLOC_MINALIGN, SLAB_HWCACHE_ALIGN, NULL); > + if (bslab->slab) { > + bslab->slab_ref = 1; > + bslab->slab_size = size; > + } else { > + kfree(bslab); > + bslab = NULL; > + } > + return bslab; > +}
I'd simply this to:
if (!bslab->slab) { kfree(bslab); return NULL; }
bslab->slab_ref = 1; bslab->slab_size = size; return bslab; }
> > static struct kmem_cache *bio_find_or_create_slab(unsigned int extra_size) > { > unsigned int sz = sizeof(struct bio) + extra_size; > struct kmem_cache *slab = NULL; > + struct bio_slab *bslab; > > mutex_lock(&bio_slab_lock); > + bslab = xa_load(&bio_slabs, sz); > + if (bslab) { > + slab = bslab->slab; > + bslab->slab_ref++; > + } else { > + bslab = create_bio_slab(sz); > + if(bslab && !xa_err(xa_store(&bio_slabs, sz, bslab, > + GFP_KERNEL)))
Missing whitespace after the "if"
But more importantly, I'd expect the xa_store to go into create_bio_slab to make the code a little more readable and to consolidate the error handling. Also we really shouldn't need the slab local variable in this function.
> static void bio_put_slab(struct bio_set *bs) > { > struct bio_slab *bslab = NULL; > + unsigned int slab_size = bs->front_pad + sizeof(struct bio) + > + BIO_INLINE_VECS * sizeof(struct bio_vec);
This calculation would look nice factored out into a little helper with a comment explaining it.
|  |