diff mbox series

[V3,1/6] block: manage bio slab cache by xarray

Message ID 20210111030557.4154161-2-ming.lei@redhat.com (mailing list archive)
State New, archived
Headers show
Series block: improvement on bioset & bvec allocation | expand

Commit Message

Ming Lei Jan. 11, 2021, 3:05 a.m. UTC 
Managing bio slab cache via xarray by using slab cache size as xarray
index, and storing 'struct bio_slab' instance into xarray.

So code is simplified a lot, meantime it becomes more readable than before.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/bio.c | 116 ++++++++++++++++++++++------------------------------
 1 file changed, 49 insertions(+), 67 deletions(-)

Comments

Pavel Begunkov Jan. 11, 2021, 4:39 a.m. UTC | #1 
On 11/01/2021 03:05, Ming Lei wrote:
> Managing bio slab cache via xarray by using slab cache size as xarray
> index, and storing 'struct bio_slab' instance into xarray.
> 
> So code is simplified a lot, meantime it becomes more readable than before.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
Tested-by: Pavel Begunkov <asml.silence@gmail.com>
Hannes Reinecke Jan. 11, 2021, 6:57 a.m. UTC | #2 
On 1/11/21 4:05 AM, Ming Lei wrote:
> Managing bio slab cache via xarray by using slab cache size as xarray
> index, and storing 'struct bio_slab' instance into xarray.
> 
> So code is simplified a lot, meantime it becomes more readable than before.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>   block/bio.c | 116 ++++++++++++++++++++++------------------------------
>   1 file changed, 49 insertions(+), 67 deletions(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index 1f2cc1fbe283..cfa0e9db30e0 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -19,6 +19,7 @@
>   #include <linux/highmem.h>
>   #include <linux/sched/sysctl.h>
>   #include <linux/blk-crypto.h>
> +#include <linux/xarray.h>
>   
>   #include <trace/events/block.h>
>   #include "blk.h"
> @@ -58,89 +59,80 @@ struct bio_slab {
>   	char name[8];
>   };
>   static DEFINE_MUTEX(bio_slab_lock);
> -static struct bio_slab *bio_slabs;
> -static unsigned int bio_slab_nr, bio_slab_max;
> +static DEFINE_XARRAY(bio_slabs);
>   
> -static struct kmem_cache *bio_find_or_create_slab(unsigned int extra_size)
> +static struct bio_slab *create_bio_slab(unsigned int size)
>   {
> -	unsigned int sz = sizeof(struct bio) + extra_size;
> -	struct kmem_cache *slab = NULL;
> -	struct bio_slab *bslab, *new_bio_slabs;
> -	unsigned int new_bio_slab_max;
> -	unsigned int i, entry = -1;
> +	struct bio_slab *bslab = kzalloc(sizeof(*bslab), GFP_KERNEL);
>   
> -	mutex_lock(&bio_slab_lock);
> +	if (!bslab)
> +		return NULL;
>   
> -	i = 0;
> -	while (i < bio_slab_nr) {
> -		bslab = &bio_slabs[i];
> +	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)
> +		goto fail_alloc_slab;
>   
> -		if (!bslab->slab && entry == -1)
> -			entry = i;
> -		else if (bslab->slab_size == sz) {
> -			slab = bslab->slab;
> -			bslab->slab_ref++;
> -			break;
> -		}
> -		i++;
> -	}
> +	bslab->slab_ref = 1;
> +	bslab->slab_size = size;
>   

Why don't you use a kref here?

Otherwise:

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
diff mbox series

Patch

diff --git a/block/bio.c b/block/bio.c
index 1f2cc1fbe283..cfa0e9db30e0 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -19,6 +19,7 @@ 
 #include <linux/highmem.h>
 #include <linux/sched/sysctl.h>
 #include <linux/blk-crypto.h>
+#include <linux/xarray.h>
 
 #include <trace/events/block.h>
 #include "blk.h"
@@ -58,89 +59,80 @@  struct bio_slab {
 	char name[8];
 };
 static DEFINE_MUTEX(bio_slab_lock);
-static struct bio_slab *bio_slabs;
-static unsigned int bio_slab_nr, bio_slab_max;
+static DEFINE_XARRAY(bio_slabs);
 
-static struct kmem_cache *bio_find_or_create_slab(unsigned int extra_size)
+static struct bio_slab *create_bio_slab(unsigned int size)
 {
-	unsigned int sz = sizeof(struct bio) + extra_size;
-	struct kmem_cache *slab = NULL;
-	struct bio_slab *bslab, *new_bio_slabs;
-	unsigned int new_bio_slab_max;
-	unsigned int i, entry = -1;
+	struct bio_slab *bslab = kzalloc(sizeof(*bslab), GFP_KERNEL);
 
-	mutex_lock(&bio_slab_lock);
+	if (!bslab)
+		return NULL;
 
-	i = 0;
-	while (i < bio_slab_nr) {
-		bslab = &bio_slabs[i];
+	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)
+		goto fail_alloc_slab;
 
-		if (!bslab->slab && entry == -1)
-			entry = i;
-		else if (bslab->slab_size == sz) {
-			slab = bslab->slab;
-			bslab->slab_ref++;
-			break;
-		}
-		i++;
-	}
+	bslab->slab_ref = 1;
+	bslab->slab_size = size;
 
-	if (slab)
-		goto out_unlock;
-
-	if (bio_slab_nr == bio_slab_max && entry == -1) {
-		new_bio_slab_max = bio_slab_max << 1;
-		new_bio_slabs = krealloc(bio_slabs,
-					 new_bio_slab_max * sizeof(struct bio_slab),
-					 GFP_KERNEL);
-		if (!new_bio_slabs)
-			goto out_unlock;
-		bio_slab_max = new_bio_slab_max;
-		bio_slabs = new_bio_slabs;
-	}
-	if (entry == -1)
-		entry = bio_slab_nr++;
+	if (!xa_err(xa_store(&bio_slabs, size, bslab, GFP_KERNEL)))
+		return bslab;
 
-	bslab = &bio_slabs[entry];
+	kmem_cache_destroy(bslab->slab);
 
-	snprintf(bslab->name, sizeof(bslab->name), "bio-%d", entry);
-	slab = kmem_cache_create(bslab->name, sz, ARCH_KMALLOC_MINALIGN,
-				 SLAB_HWCACHE_ALIGN, NULL);
-	if (!slab)
-		goto out_unlock;
+fail_alloc_slab:
+	kfree(bslab);
+	return NULL;
+}
 
-	bslab->slab = slab;
-	bslab->slab_ref = 1;
-	bslab->slab_size = sz;
-out_unlock:
+static inline unsigned int bs_bio_slab_size(struct bio_set *bs)
+{
+	return bs->front_pad + sizeof(struct bio) +
+		BIO_INLINE_VECS * sizeof(struct bio_vec);
+}
+
+static struct kmem_cache *bio_find_or_create_slab(struct bio_set *bs)
+{
+	unsigned int size = bs_bio_slab_size(bs);
+	struct bio_slab *bslab;
+
+	mutex_lock(&bio_slab_lock);
+	bslab = xa_load(&bio_slabs, size);
+	if (bslab)
+		bslab->slab_ref++;
+	else
+		bslab = create_bio_slab(size);
 	mutex_unlock(&bio_slab_lock);
-	return slab;
+
+	if (bslab)
+		return bslab->slab;
+	return NULL;
 }
 
 static void bio_put_slab(struct bio_set *bs)
 {
 	struct bio_slab *bslab = NULL;
-	unsigned int i;
+	unsigned int slab_size = bs_bio_slab_size(bs);
 
 	mutex_lock(&bio_slab_lock);
 
-	for (i = 0; i < bio_slab_nr; i++) {
-		if (bs->bio_slab == bio_slabs[i].slab) {
-			bslab = &bio_slabs[i];
-			break;
-		}
-	}
-
+	bslab = xa_load(&bio_slabs, slab_size);
 	if (WARN(!bslab, KERN_ERR "bio: unable to find slab!\n"))
 		goto out;
 
+	WARN_ON_ONCE(bslab->slab != bs->bio_slab);
+
 	WARN_ON(!bslab->slab_ref);
 
 	if (--bslab->slab_ref)
 		goto out;
 
+	xa_erase(&bio_slabs, slab_size);
+
 	kmem_cache_destroy(bslab->slab);
-	bslab->slab = NULL;
+	kfree(bslab);
 
 out:
 	mutex_unlock(&bio_slab_lock);
@@ -1579,15 +1571,13 @@  int bioset_init(struct bio_set *bs,
 		unsigned int front_pad,
 		int flags)
 {
-	unsigned int back_pad = BIO_INLINE_VECS * sizeof(struct bio_vec);
-
 	bs->front_pad = front_pad;
 
 	spin_lock_init(&bs->rescue_lock);
 	bio_list_init(&bs->rescue_list);
 	INIT_WORK(&bs->rescue_work, bio_alloc_rescue);
 
-	bs->bio_slab = bio_find_or_create_slab(front_pad + back_pad);
+	bs->bio_slab = bio_find_or_create_slab(bs);
 	if (!bs->bio_slab)
 		return -ENOMEM;
 
@@ -1651,16 +1641,8 @@  static void __init biovec_init_slabs(void)
 
 static int __init init_bio(void)
 {
-	bio_slab_max = 2;
-	bio_slab_nr = 0;
-	bio_slabs = kcalloc(bio_slab_max, sizeof(struct bio_slab),
-			    GFP_KERNEL);
-
 	BUILD_BUG_ON(BIO_FLAG_LAST > BVEC_POOL_OFFSET);
 
-	if (!bio_slabs)
-		panic("bio: can't allocate bios\n");
-
 	bio_integrity_init();
 	biovec_init_slabs();