Re: [PATCH 02/10] block: Convert bio_set to mempool_init()

[Kent Overstreet]

On Fri, May 18, 2018 at 09:20:28AM -0700, Christoph Hellwig wrote:
> On Tue, May 08, 2018 at 09:33:50PM -0400, Kent Overstreet wrote:
> > Minor performance improvement by getting rid of pointer indirections
> > from allocation/freeing fastpaths.
> 
> Can you please also send a long conversion for the remaining
> few bioset_create users?  It would be rather silly to keep two
> almost the same interfaces around for just about two hand full
> of users.

Yeah, I can do that

Re: [PATCH 02/10] block: Convert bio_set to mempool_init()

[Christoph Hellwig]

On Fri, May 18, 2018 at 09:20:28AM -0700, Christoph Hellwig wrote:
> On Tue, May 08, 2018 at 09:33:50PM -0400, Kent Overstreet wrote:
> > Minor performance improvement by getting rid of pointer indirections
> > from allocation/freeing fastpaths.
> 
> Can you please also send a long conversion for the remaining
> few bioset_create users?  It would be rather silly to keep two
> almost the same interfaces around for just about two hand full
> of users.

This comment was ment in reply to the next patch, sorry.

Re: [PATCH 02/10] block: Convert bio_set to mempool_init()

[Christoph Hellwig]

On Tue, May 08, 2018 at 09:33:50PM -0400, Kent Overstreet wrote:
> Minor performance improvement by getting rid of pointer indirections
> from allocation/freeing fastpaths.

Can you please also send a long conversion for the remaining
few bioset_create users?  It would be rather silly to keep two
almost the same interfaces around for just about two hand full
of users.

Re: [PATCH 02/10] block: Convert bio_set to mempool_init()

[Johannes Thumshirn]

On Fri, May 18, 2018 at 03:49:01AM -0400, Kent Overstreet wrote:
> Minor performance improvement by getting rid of pointer indirections
> from allocation/freeing fastpaths.

Reviewed-by: Johannes Thumshirn 

Although I'd prefer numbers in the changelog when claiming a
performance improvement.

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 02/10] block: Convert bio_set to mempool_init()

[Kent Overstreet]

Minor performance improvement by getting rid of pointer indirections
from allocation/freeing fastpaths.

Signed-off-by: Kent Overstreet 
---
 block/bio-integrity.c | 29 ++---
 block/bio.c   | 36 +---
 include/linux/bio.h   | 10 +-
 3 files changed, 36 insertions(+), 39 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 9cfdd6c83b..add7c7c853 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -56,12 +56,12 @@ struct bio_integrity_payload *bio_integrity_alloc(struct 
bio *bio,
struct bio_set *bs = bio->bi_pool;
unsigned inline_vecs;
 
-   if (!bs || !bs->bio_integrity_pool) {
+   if (!bs || !mempool_initialized(>bio_integrity_pool)) {
bip = kmalloc(sizeof(struct bio_integrity_payload) +
  sizeof(struct bio_vec) * nr_vecs, gfp_mask);
inline_vecs = nr_vecs;
} else {
-   bip = mempool_alloc(bs->bio_integrity_pool, gfp_mask);
+   bip = mempool_alloc(>bio_integrity_pool, gfp_mask);
inline_vecs = BIP_INLINE_VECS;
}
 
@@ -74,7 +74,7 @@ struct bio_integrity_payload *bio_integrity_alloc(struct bio 
*bio,
unsigned long idx = 0;
 
bip->bip_vec = bvec_alloc(gfp_mask, nr_vecs, ,
- bs->bvec_integrity_pool);
+ >bvec_integrity_pool);
if (!bip->bip_vec)
goto err;
bip->bip_max_vcnt = bvec_nr_vecs(idx);
@@ -90,7 +90,7 @@ struct bio_integrity_payload *bio_integrity_alloc(struct bio 
*bio,
 
return bip;
 err:
-   mempool_free(bip, bs->bio_integrity_pool);
+   mempool_free(bip, >bio_integrity_pool);
return ERR_PTR(-ENOMEM);
 }
 EXPORT_SYMBOL(bio_integrity_alloc);
@@ -111,10 +111,10 @@ static void bio_integrity_free(struct bio *bio)
kfree(page_address(bip->bip_vec->bv_page) +
  bip->bip_vec->bv_offset);
 
-   if (bs && bs->bio_integrity_pool) {
-   bvec_free(bs->bvec_integrity_pool, bip->bip_vec, bip->bip_slab);
+   if (bs && mempool_initialized(>bio_integrity_pool)) {
+   bvec_free(>bvec_integrity_pool, bip->bip_vec, 
bip->bip_slab);
 
-   mempool_free(bip, bs->bio_integrity_pool);
+   mempool_free(bip, >bio_integrity_pool);
} else {
kfree(bip);
}
@@ -465,16 +465,15 @@ EXPORT_SYMBOL(bio_integrity_clone);
 
 int bioset_integrity_create(struct bio_set *bs, int pool_size)
 {
-   if (bs->bio_integrity_pool)
+   if (mempool_initialized(>bio_integrity_pool))
return 0;
 
-   bs->bio_integrity_pool = mempool_create_slab_pool(pool_size, bip_slab);
-   if (!bs->bio_integrity_pool)
+   if (mempool_init_slab_pool(>bio_integrity_pool,
+  pool_size, bip_slab))
return -1;
 
-   bs->bvec_integrity_pool = biovec_create_pool(pool_size);
-   if (!bs->bvec_integrity_pool) {
-   mempool_destroy(bs->bio_integrity_pool);
+   if (biovec_init_pool(>bvec_integrity_pool, pool_size)) {
+   mempool_exit(>bio_integrity_pool);
return -1;
}
 
@@ -484,8 +483,8 @@ EXPORT_SYMBOL(bioset_integrity_create);
 
 void bioset_integrity_free(struct bio_set *bs)
 {
-   mempool_destroy(bs->bio_integrity_pool);
-   mempool_destroy(bs->bvec_integrity_pool);
+   mempool_exit(>bio_integrity_pool);
+   mempool_exit(>bvec_integrity_pool);
 }
 EXPORT_SYMBOL(bioset_integrity_free);
 
diff --git a/block/bio.c b/block/bio.c
index e1708db482..360e9bcea5 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -254,7 +254,7 @@ static void bio_free(struct bio *bio)
bio_uninit(bio);
 
if (bs) {
-   bvec_free(bs->bvec_pool, bio->bi_io_vec, BVEC_POOL_IDX(bio));
+   bvec_free(>bvec_pool, bio->bi_io_vec, BVEC_POOL_IDX(bio));
 
/*
 * If we have front padding, adjust the bio pointer before 
freeing
@@ -262,7 +262,7 @@ static void bio_free(struct bio *bio)
p = bio;
p -= bs->front_pad;
 
-   mempool_free(p, bs->bio_pool);
+   mempool_free(p, >bio_pool);
} else {
/* Bio was allocated by bio_kmalloc() */
kfree(bio);
@@ -454,7 +454,8 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, unsigned int 
nr_iovecs,
inline_vecs = nr_iovecs;
} else {
/* should not use nobvec bioset for nr_iovecs > 0 */
-   if (WARN_ON_ONCE(!bs->bvec_pool && nr_iovecs > 0))
+   if (WARN_ON_ONCE(!mempool_initialized(>bvec_pool) &&
+nr_iovecs > 0))
return NULL;
/*
 * 

[PATCH 02/10] block: Convert bio_set to mempool_init()

[Kent Overstreet]

Minor performance improvement by getting rid of pointer indirections
from allocation/freeing fastpaths.

Signed-off-by: Kent Overstreet 
---
 block/bio-integrity.c | 29 ++---
 block/bio.c   | 36 +---
 include/linux/bio.h   | 10 +-
 3 files changed, 36 insertions(+), 39 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 9cfdd6c83b..add7c7c853 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -56,12 +56,12 @@ struct bio_integrity_payload *bio_integrity_alloc(struct 
bio *bio,
struct bio_set *bs = bio->bi_pool;
unsigned inline_vecs;
 
-   if (!bs || !bs->bio_integrity_pool) {
+   if (!bs || !mempool_initialized(>bio_integrity_pool)) {
bip = kmalloc(sizeof(struct bio_integrity_payload) +
  sizeof(struct bio_vec) * nr_vecs, gfp_mask);
inline_vecs = nr_vecs;
} else {
-   bip = mempool_alloc(bs->bio_integrity_pool, gfp_mask);
+   bip = mempool_alloc(>bio_integrity_pool, gfp_mask);
inline_vecs = BIP_INLINE_VECS;
}
 
@@ -74,7 +74,7 @@ struct bio_integrity_payload *bio_integrity_alloc(struct bio 
*bio,
unsigned long idx = 0;
 
bip->bip_vec = bvec_alloc(gfp_mask, nr_vecs, ,
- bs->bvec_integrity_pool);
+ >bvec_integrity_pool);
if (!bip->bip_vec)
goto err;
bip->bip_max_vcnt = bvec_nr_vecs(idx);
@@ -90,7 +90,7 @@ struct bio_integrity_payload *bio_integrity_alloc(struct bio 
*bio,
 
return bip;
 err:
-   mempool_free(bip, bs->bio_integrity_pool);
+   mempool_free(bip, >bio_integrity_pool);
return ERR_PTR(-ENOMEM);
 }
 EXPORT_SYMBOL(bio_integrity_alloc);
@@ -111,10 +111,10 @@ static void bio_integrity_free(struct bio *bio)
kfree(page_address(bip->bip_vec->bv_page) +
  bip->bip_vec->bv_offset);
 
-   if (bs && bs->bio_integrity_pool) {
-   bvec_free(bs->bvec_integrity_pool, bip->bip_vec, bip->bip_slab);
+   if (bs && mempool_initialized(>bio_integrity_pool)) {
+   bvec_free(>bvec_integrity_pool, bip->bip_vec, 
bip->bip_slab);
 
-   mempool_free(bip, bs->bio_integrity_pool);
+   mempool_free(bip, >bio_integrity_pool);
} else {
kfree(bip);
}
@@ -465,16 +465,15 @@ EXPORT_SYMBOL(bio_integrity_clone);
 
 int bioset_integrity_create(struct bio_set *bs, int pool_size)
 {
-   if (bs->bio_integrity_pool)
+   if (mempool_initialized(>bio_integrity_pool))
return 0;
 
-   bs->bio_integrity_pool = mempool_create_slab_pool(pool_size, bip_slab);
-   if (!bs->bio_integrity_pool)
+   if (mempool_init_slab_pool(>bio_integrity_pool,
+  pool_size, bip_slab))
return -1;
 
-   bs->bvec_integrity_pool = biovec_create_pool(pool_size);
-   if (!bs->bvec_integrity_pool) {
-   mempool_destroy(bs->bio_integrity_pool);
+   if (biovec_init_pool(>bvec_integrity_pool, pool_size)) {
+   mempool_exit(>bio_integrity_pool);
return -1;
}
 
@@ -484,8 +483,8 @@ EXPORT_SYMBOL(bioset_integrity_create);
 
 void bioset_integrity_free(struct bio_set *bs)
 {
-   mempool_destroy(bs->bio_integrity_pool);
-   mempool_destroy(bs->bvec_integrity_pool);
+   mempool_exit(>bio_integrity_pool);
+   mempool_exit(>bvec_integrity_pool);
 }
 EXPORT_SYMBOL(bioset_integrity_free);
 
diff --git a/block/bio.c b/block/bio.c
index e1708db482..360e9bcea5 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -254,7 +254,7 @@ static void bio_free(struct bio *bio)
bio_uninit(bio);
 
if (bs) {
-   bvec_free(bs->bvec_pool, bio->bi_io_vec, BVEC_POOL_IDX(bio));
+   bvec_free(>bvec_pool, bio->bi_io_vec, BVEC_POOL_IDX(bio));
 
/*
 * If we have front padding, adjust the bio pointer before 
freeing
@@ -262,7 +262,7 @@ static void bio_free(struct bio *bio)
p = bio;
p -= bs->front_pad;
 
-   mempool_free(p, bs->bio_pool);
+   mempool_free(p, >bio_pool);
} else {
/* Bio was allocated by bio_kmalloc() */
kfree(bio);
@@ -454,7 +454,8 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, unsigned int 
nr_iovecs,
inline_vecs = nr_iovecs;
} else {
/* should not use nobvec bioset for nr_iovecs > 0 */
-   if (WARN_ON_ONCE(!bs->bvec_pool && nr_iovecs > 0))
+   if (WARN_ON_ONCE(!mempool_initialized(>bvec_pool) &&
+nr_iovecs > 0))
return NULL;
/*
 *