[PATCH V3 0/6] block: improvement on bioset & bvec allocation

[PATCH V3 0/6] block: improvement on bioset & bvec allocation

[Ming Lei]

Hello Jens,

All are bioset / bvec improvement, and most of them are quite
straightforward.

V3:
	- share two line code via goto with one label, only patch 1 is
	  changed

V2:
	- patch style change, most is in patch 1
	- commit log change

Ming Lei (6):
  block: manage bio slab cache by xarray
  block: don't pass BIOSET_NEED_BVECS for q->bio_split
  block: don't allocate inline bvecs if this bioset needn't bvecs
  block: set .bi_max_vecs as actual allocated vector number
  block: move three bvec helpers declaration into private helper
  bcache: don't pass BIOSET_NEED_BVECS for the 'bio_set' embedded in
    'cache_set'

 block/bio.c               | 122 +++++++++++++++++---------------------
 block/blk-core.c          |   2 +-
 block/blk.h               |   4 ++
 drivers/md/bcache/super.c |   2 +-
 include/linux/bio.h       |   4 +-
 5 files changed, 61 insertions(+), 73 deletions(-)

-- 
2.28.0

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

[Ming Lei]

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;
 
-	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();
 
-- 
2.28.0

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

[Pavel Begunkov]

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>

-- 
Pavel Begunkov

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

[Hannes Reinecke]

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
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

[PATCH V3 2/6] block: don't pass BIOSET_NEED_BVECS for q->bio_split

[Ming Lei]

q->bio_split is only used by bio_split() for fast cloning bio, and no
need to allocate bvecs, so remove this flag.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 7663a9b94b80..00d415be74e6 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -531,7 +531,7 @@ struct request_queue *blk_alloc_queue(int node_id)
 	if (q->id < 0)
 		goto fail_q;
 
-	ret = bioset_init(&q->bio_split, BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS);
+	ret = bioset_init(&q->bio_split, BIO_POOL_SIZE, 0, 0);
 	if (ret)
 		goto fail_id;
 
-- 
2.28.0

Re: [PATCH V3 2/6] block: don't pass BIOSET_NEED_BVECS for q->bio_split

[Pavel Begunkov]

On 11/01/2021 03:05, Ming Lei wrote:
> q->bio_split is only used by bio_split() for fast cloning bio, and no
> need to allocate bvecs, so remove this flag.
> 
> 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>

> ---
>  block/blk-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 7663a9b94b80..00d415be74e6 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -531,7 +531,7 @@ struct request_queue *blk_alloc_queue(int node_id)
>  	if (q->id < 0)
>  		goto fail_q;
>  
> -	ret = bioset_init(&q->bio_split, BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS);
> +	ret = bioset_init(&q->bio_split, BIO_POOL_SIZE, 0, 0);
>  	if (ret)
>  		goto fail_id;
>  
> 

-- 
Pavel Begunkov

Re: [PATCH V3 2/6] block: don't pass BIOSET_NEED_BVECS for q->bio_split

[Hannes Reinecke]

On 1/11/21 4:05 AM, Ming Lei wrote:
> q->bio_split is only used by bio_split() for fast cloning bio, and no
> need to allocate bvecs, so remove this flag.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>   block/blk-core.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 7663a9b94b80..00d415be74e6 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -531,7 +531,7 @@ struct request_queue *blk_alloc_queue(int node_id)
>   	if (q->id < 0)
>   		goto fail_q;
>   
> -	ret = bioset_init(&q->bio_split, BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS);
> +	ret = bioset_init(&q->bio_split, BIO_POOL_SIZE, 0, 0);
>   	if (ret)
>   		goto fail_id;
>   
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

[PATCH V3 3/6] block: don't allocate inline bvecs if this bioset needn't bvecs

[Ming Lei]

The inline bvecs won't be used if user needn't bvecs by not passing
BIOSET_NEED_BVECS, so don't allocate bvecs in this situation.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/bio.c         | 7 +++++--
 include/linux/bio.h | 1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index cfa0e9db30e0..496aa5938f79 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -89,8 +89,7 @@ static struct bio_slab *create_bio_slab(unsigned int size)
 
 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);
+	return bs->front_pad + sizeof(struct bio) + bs->back_pad;
 }
 
 static struct kmem_cache *bio_find_or_create_slab(struct bio_set *bs)
@@ -1572,6 +1571,10 @@ int bioset_init(struct bio_set *bs,
 		int flags)
 {
 	bs->front_pad = front_pad;
+	if (flags & BIOSET_NEED_BVECS)
+		bs->back_pad = BIO_INLINE_VECS * sizeof(struct bio_vec);
+	else
+		bs->back_pad = 0;
 
 	spin_lock_init(&bs->rescue_lock);
 	bio_list_init(&bs->rescue_list);
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 1edda614f7ce..f606eb1e556f 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -703,6 +703,7 @@ struct bio_set {
 	mempool_t bvec_integrity_pool;
 #endif
 
+	unsigned int back_pad;
 	/*
 	 * Deadlock avoidance for stacking block drivers: see comments in
 	 * bio_alloc_bioset() for details
-- 
2.28.0

Re: [PATCH V3 3/6] block: don't allocate inline bvecs if this bioset needn't bvecs

[Pavel Begunkov]

On 11/01/2021 03:05, Ming Lei wrote:
> The inline bvecs won't be used if user needn't bvecs by not passing
> BIOSET_NEED_BVECS, so don't allocate bvecs in this situation.
> 
> 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>

> ---
>  block/bio.c         | 7 +++++--
>  include/linux/bio.h | 1 +
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index cfa0e9db30e0..496aa5938f79 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -89,8 +89,7 @@ static struct bio_slab *create_bio_slab(unsigned int size)
>  
>  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);
> +	return bs->front_pad + sizeof(struct bio) + bs->back_pad;
>  }
>  
>  static struct kmem_cache *bio_find_or_create_slab(struct bio_set *bs)
> @@ -1572,6 +1571,10 @@ int bioset_init(struct bio_set *bs,
>  		int flags)
>  {
>  	bs->front_pad = front_pad;
> +	if (flags & BIOSET_NEED_BVECS)
> +		bs->back_pad = BIO_INLINE_VECS * sizeof(struct bio_vec);
> +	else
> +		bs->back_pad = 0;
>  
>  	spin_lock_init(&bs->rescue_lock);
>  	bio_list_init(&bs->rescue_list);
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index 1edda614f7ce..f606eb1e556f 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -703,6 +703,7 @@ struct bio_set {
>  	mempool_t bvec_integrity_pool;
>  #endif
>  
> +	unsigned int back_pad;
>  	/*
>  	 * Deadlock avoidance for stacking block drivers: see comments in
>  	 * bio_alloc_bioset() for details
> 

-- 
Pavel Begunkov

Re: [PATCH V3 3/6] block: don't allocate inline bvecs if this bioset needn't bvecs

[Hannes Reinecke]

On 1/11/21 4:05 AM, Ming Lei wrote:
> The inline bvecs won't be used if user needn't bvecs by not passing
> BIOSET_NEED_BVECS, so don't allocate bvecs in this situation.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>   block/bio.c         | 7 +++++--
>   include/linux/bio.h | 1 +
>   2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index cfa0e9db30e0..496aa5938f79 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -89,8 +89,7 @@ static struct bio_slab *create_bio_slab(unsigned int size)
>   
>   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);
> +	return bs->front_pad + sizeof(struct bio) + bs->back_pad;
>   }
>   
>   static struct kmem_cache *bio_find_or_create_slab(struct bio_set *bs)
> @@ -1572,6 +1571,10 @@ int bioset_init(struct bio_set *bs,
>   		int flags)
>   {
>   	bs->front_pad = front_pad;
> +	if (flags & BIOSET_NEED_BVECS)
> +		bs->back_pad = BIO_INLINE_VECS * sizeof(struct bio_vec);
> +	else
> +		bs->back_pad = 0;
>   
>   	spin_lock_init(&bs->rescue_lock);
>   	bio_list_init(&bs->rescue_list);
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index 1edda614f7ce..f606eb1e556f 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -703,6 +703,7 @@ struct bio_set {
>   	mempool_t bvec_integrity_pool;
>   #endif
>   
> +	unsigned int back_pad;
>   	/*
>   	 * Deadlock avoidance for stacking block drivers: see comments in
>   	 * bio_alloc_bioset() for details
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

[PATCH V3 4/6] block: set .bi_max_vecs as actual allocated vector number

[Ming Lei]

bvec_alloc() may allocate more bio vectors than requested, so set
.bi_max_vecs as actual allocated vector number, instead of the requested
number. This way can help fs build bigger bio because new bio often won't
be allocated until the current one becomes full.

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

diff --git a/block/bio.c b/block/bio.c
index 496aa5938f79..37e3f2d9df99 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -505,12 +505,13 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, unsigned int nr_iovecs,
 			goto err_free;
 
 		bio->bi_flags |= idx << BVEC_POOL_OFFSET;
+		bio->bi_max_vecs = bvec_nr_vecs(idx);
 	} else if (nr_iovecs) {
 		bvl = bio->bi_inline_vecs;
+		bio->bi_max_vecs = inline_vecs;
 	}
 
 	bio->bi_pool = bs;
-	bio->bi_max_vecs = nr_iovecs;
 	bio->bi_io_vec = bvl;
 	return bio;
 
-- 
2.28.0

Re: [PATCH V3 4/6] block: set .bi_max_vecs as actual allocated vector number

[Pavel Begunkov]

On 11/01/2021 03:05, Ming Lei wrote:
> bvec_alloc() may allocate more bio vectors than requested, so set
> .bi_max_vecs as actual allocated vector number, instead of the requested
> number. This way can help fs build bigger bio because new bio often won't
> be allocated until the current one becomes full.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>

> ---
>  block/bio.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index 496aa5938f79..37e3f2d9df99 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -505,12 +505,13 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, unsigned int nr_iovecs,
>  			goto err_free;
>  
>  		bio->bi_flags |= idx << BVEC_POOL_OFFSET;
> +		bio->bi_max_vecs = bvec_nr_vecs(idx);
>  	} else if (nr_iovecs) {
>  		bvl = bio->bi_inline_vecs;
> +		bio->bi_max_vecs = inline_vecs;
>  	}
>  
>  	bio->bi_pool = bs;
> -	bio->bi_max_vecs = nr_iovecs;
>  	bio->bi_io_vec = bvl;
>  	return bio;
>  
> 

-- 
Pavel Begunkov

Re: [PATCH V3 4/6] block: set .bi_max_vecs as actual allocated vector number

[Hannes Reinecke]

On 1/11/21 4:05 AM, Ming Lei wrote:
> bvec_alloc() may allocate more bio vectors than requested, so set
> .bi_max_vecs as actual allocated vector number, instead of the requested
> number. This way can help fs build bigger bio because new bio often won't
> be allocated until the current one becomes full.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>   block/bio.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index 496aa5938f79..37e3f2d9df99 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -505,12 +505,13 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, unsigned int nr_iovecs,
>   			goto err_free;
>   
>   		bio->bi_flags |= idx << BVEC_POOL_OFFSET;
> +		bio->bi_max_vecs = bvec_nr_vecs(idx);
>   	} else if (nr_iovecs) {
>   		bvl = bio->bi_inline_vecs;
> +		bio->bi_max_vecs = inline_vecs;
>   	}
>   
>   	bio->bi_pool = bs;
> -	bio->bi_max_vecs = nr_iovecs;
>   	bio->bi_io_vec = bvl;
>   	return bio;
>   
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

[PATCH V3 5/6] block: move three bvec helpers declaration into private helper

[Ming Lei]

bvec_alloc(), bvec_free() and bvec_nr_vecs() are only used inside block
layer core functions, no need to declare them in public header.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk.h         | 4 ++++
 include/linux/bio.h | 3 ---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/blk.h b/block/blk.h
index 7550364c326c..a21a35c4a3e4 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -55,6 +55,10 @@ void blk_free_flush_queue(struct blk_flush_queue *q);
 
 void blk_freeze_queue(struct request_queue *q);
 
+struct bio_vec *bvec_alloc(gfp_t, int, unsigned long *, mempool_t *);
+void bvec_free(mempool_t *, struct bio_vec *, unsigned int);
+unsigned int bvec_nr_vecs(unsigned short idx);
+
 static inline bool biovec_phys_mergeable(struct request_queue *q,
 		struct bio_vec *vec1, struct bio_vec *vec2)
 {
diff --git a/include/linux/bio.h b/include/linux/bio.h
index f606eb1e556f..70914dd6a70d 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -478,9 +478,6 @@ static inline void zero_fill_bio(struct bio *bio)
 	zero_fill_bio_iter(bio, bio->bi_iter);
 }
 
-extern struct bio_vec *bvec_alloc(gfp_t, int, unsigned long *, mempool_t *);
-extern void bvec_free(mempool_t *, struct bio_vec *, unsigned int);
-extern unsigned int bvec_nr_vecs(unsigned short idx);
 extern const char *bio_devname(struct bio *bio, char *buffer);
 
 #define bio_set_dev(bio, bdev) 			\
-- 
2.28.0

Re: [PATCH V3 5/6] block: move three bvec helpers declaration into private helper

[Pavel Begunkov]

On 11/01/2021 03:05, Ming Lei wrote:
> bvec_alloc(), bvec_free() and bvec_nr_vecs() are only used inside block
> layer core functions, no need to declare them in public header.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>

> ---
>  block/blk.h         | 4 ++++
>  include/linux/bio.h | 3 ---
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/block/blk.h b/block/blk.h
> index 7550364c326c..a21a35c4a3e4 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -55,6 +55,10 @@ void blk_free_flush_queue(struct blk_flush_queue *q);
>  
>  void blk_freeze_queue(struct request_queue *q);
>  
> +struct bio_vec *bvec_alloc(gfp_t, int, unsigned long *, mempool_t *);
> +void bvec_free(mempool_t *, struct bio_vec *, unsigned int);
> +unsigned int bvec_nr_vecs(unsigned short idx);
> +
>  static inline bool biovec_phys_mergeable(struct request_queue *q,
>  		struct bio_vec *vec1, struct bio_vec *vec2)
>  {
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index f606eb1e556f..70914dd6a70d 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -478,9 +478,6 @@ static inline void zero_fill_bio(struct bio *bio)
>  	zero_fill_bio_iter(bio, bio->bi_iter);
>  }
>  
> -extern struct bio_vec *bvec_alloc(gfp_t, int, unsigned long *, mempool_t *);
> -extern void bvec_free(mempool_t *, struct bio_vec *, unsigned int);
> -extern unsigned int bvec_nr_vecs(unsigned short idx);
>  extern const char *bio_devname(struct bio *bio, char *buffer);
>  
>  #define bio_set_dev(bio, bdev) 			\
> 

-- 
Pavel Begunkov

Re: [PATCH V3 5/6] block: move three bvec helpers declaration into private helper

[Hannes Reinecke]

On 1/11/21 4:05 AM, Ming Lei wrote:
> bvec_alloc(), bvec_free() and bvec_nr_vecs() are only used inside block
> layer core functions, no need to declare them in public header.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>   block/blk.h         | 4 ++++
>   include/linux/bio.h | 3 ---
>   2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/block/blk.h b/block/blk.h
> index 7550364c326c..a21a35c4a3e4 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -55,6 +55,10 @@ void blk_free_flush_queue(struct blk_flush_queue *q);
>   
>   void blk_freeze_queue(struct request_queue *q);
>   
> +struct bio_vec *bvec_alloc(gfp_t, int, unsigned long *, mempool_t *);
> +void bvec_free(mempool_t *, struct bio_vec *, unsigned int);
> +unsigned int bvec_nr_vecs(unsigned short idx);
> +
>   static inline bool biovec_phys_mergeable(struct request_queue *q,
>   		struct bio_vec *vec1, struct bio_vec *vec2)
>   {
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index f606eb1e556f..70914dd6a70d 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -478,9 +478,6 @@ static inline void zero_fill_bio(struct bio *bio)
>   	zero_fill_bio_iter(bio, bio->bi_iter);
>   }
>   
> -extern struct bio_vec *bvec_alloc(gfp_t, int, unsigned long *, mempool_t *);
> -extern void bvec_free(mempool_t *, struct bio_vec *, unsigned int);
> -extern unsigned int bvec_nr_vecs(unsigned short idx);
>   extern const char *bio_devname(struct bio *bio, char *buffer);
>   
>   #define bio_set_dev(bio, bdev) 			\
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

[PATCH V3 6/6] bcache: don't pass BIOSET_NEED_BVECS for the 'bio_set' embedded in 'cache_set'

[Ming Lei]

This bioset is just for allocating bio only from bio_next_split, and it
needn't bvecs, so remove the flag.

Cc: linux-bcache@vger.kernel.org
Cc: Coly Li <colyli@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/md/bcache/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index a4752ac410dc..4102e47f43e1 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1897,7 +1897,7 @@ struct cache_set *bch_cache_set_alloc(struct cache_sb *sb)
 		goto err;
 
 	if (bioset_init(&c->bio_split, 4, offsetof(struct bbio, bio),
-			BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER))
+			BIOSET_NEED_RESCUER))
 		goto err;
 
 	c->uuids = alloc_meta_bucket_pages(GFP_KERNEL, sb);
-- 
2.28.0

Re: [PATCH V3 6/6] bcache: don't pass BIOSET_NEED_BVECS for the 'bio_set' embedded in 'cache_set'

[Coly Li]

On 1/11/21 11:05 AM, Ming Lei wrote:
> This bioset is just for allocating bio only from bio_next_split, and it
> needn't bvecs, so remove the flag.
> 
> Cc: linux-bcache@vger.kernel.org
> Cc: Coly Li <colyli@suse.de>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Acked-by: Coly Li <colyli@suse.de>

Thanks.

Coly Li

> ---
>  drivers/md/bcache/super.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index a4752ac410dc..4102e47f43e1 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -1897,7 +1897,7 @@ struct cache_set *bch_cache_set_alloc(struct cache_sb *sb)
>  		goto err;
>  
>  	if (bioset_init(&c->bio_split, 4, offsetof(struct bbio, bio),
> -			BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER))
> +			BIOSET_NEED_RESCUER))
>  		goto err;
>  
>  	c->uuids = alloc_meta_bucket_pages(GFP_KERNEL, sb);
> 

Re: [PATCH V3 6/6] bcache: don't pass BIOSET_NEED_BVECS for the 'bio_set' embedded in 'cache_set'

[Hannes Reinecke]

On 1/11/21 4:05 AM, Ming Lei wrote:
> This bioset is just for allocating bio only from bio_next_split, and it
> needn't bvecs, so remove the flag.
> 
> Cc: linux-bcache@vger.kernel.org
> Cc: Coly Li <colyli@suse.de>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>   drivers/md/bcache/super.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index a4752ac410dc..4102e47f43e1 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -1897,7 +1897,7 @@ struct cache_set *bch_cache_set_alloc(struct cache_sb *sb)
>   		goto err;
>   
>   	if (bioset_init(&c->bio_split, 4, offsetof(struct bbio, bio),
> -			BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER))
> +			BIOSET_NEED_RESCUER))
>   		goto err;
>   
>   	c->uuids = alloc_meta_bucket_pages(GFP_KERNEL, sb);
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH V3 0/6] block: improvement on bioset & bvec allocation

[Pavel Begunkov]

On 11/01/2021 03:05, Ming Lei wrote:
> Hello Jens,
> 
> All are bioset / bvec improvement, and most of them are quite
> straightforward.

That's cool. As mentioned before, removing inline bvecs brings
struct blkdev_dio from 3 cachelines to 2. And together with no-copy
bvec we can do that shrinking for all bvec based requests.

> 
> V3:
> 	- share two line code via goto with one label, only patch 1 is
> 	  changed
> 
> V2:
> 	- patch style change, most is in patch 1
> 	- commit log change
> 
> Ming Lei (6):
>   block: manage bio slab cache by xarray
>   block: don't pass BIOSET_NEED_BVECS for q->bio_split
>   block: don't allocate inline bvecs if this bioset needn't bvecs
>   block: set .bi_max_vecs as actual allocated vector number
>   block: move three bvec helpers declaration into private helper
>   bcache: don't pass BIOSET_NEED_BVECS for the 'bio_set' embedded in
>     'cache_set'
> 
>  block/bio.c               | 122 +++++++++++++++++---------------------
>  block/blk-core.c          |   2 +-
>  block/blk.h               |   4 ++
>  drivers/md/bcache/super.c |   2 +-
>  include/linux/bio.h       |   4 +-
>  5 files changed, 61 insertions(+), 73 deletions(-)
> 

-- 
Pavel Begunkov

Re: [PATCH V3 0/6] block: improvement on bioset & bvec allocation

[Ming Lei]

On Mon, Jan 11, 2021 at 11:05:51AM +0800, Ming Lei wrote:
> Hello Jens,
> 
> All are bioset / bvec improvement, and most of them are quite
> straightforward.
> 
> V3:
> 	- share two line code via goto with one label, only patch 1 is
> 	  changed
> 
> V2:
> 	- patch style change, most is in patch 1
> 	- commit log change
> 
> Ming Lei (6):
>   block: manage bio slab cache by xarray
>   block: don't pass BIOSET_NEED_BVECS for q->bio_split
>   block: don't allocate inline bvecs if this bioset needn't bvecs
>   block: set .bi_max_vecs as actual allocated vector number
>   block: move three bvec helpers declaration into private helper
>   bcache: don't pass BIOSET_NEED_BVECS for the 'bio_set' embedded in
>     'cache_set'
> 
>  block/bio.c               | 122 +++++++++++++++++---------------------
>  block/blk-core.c          |   2 +-
>  block/blk.h               |   4 ++
>  drivers/md/bcache/super.c |   2 +-
>  include/linux/bio.h       |   4 +-
>  5 files changed, 61 insertions(+), 73 deletions(-)
> 
> -- 
> 2.28.0
> 

Hello Jens,

Can you queue this patchset for 5.12?


Thanks,
Ming

Re: [PATCH V3 0/6] block: improvement on bioset & bvec allocation

[Jens Axboe]

On 1/10/21 8:05 PM, Ming Lei wrote:
> Hello Jens,
> 
> All are bioset / bvec improvement, and most of them are quite
> straightforward.

Queued up, thanks.

-- 
Jens Axboe