Re: [dm-devel] [PATCH v2 01/26] block: Convert integrity to bvec_alloc_bs(), and a bugfix
On Wed, Sep 12, 2012 at 03:39:18PM -0400, Martin K. Petersen wrote: > > "Kent" == Kent Overstreet writes: > > Kent, > > Kent> To fix the bug first, I'd have to reorder struct bio_pair and then > Kent> just delete two lines of code from bio_integrity_split(). But the > Kent> reordering is unnecessary with the refactoring. > > Well, a bug is a bug and the fix needs to go into stable. So we will > need a patch that does not depend on your changes. Alright, good point. > I don't have a problem with adding a pointer so clones can point to the > parent's vector. But embedding the vector into the bip was a feature. > If you check the git log you'll see that originally I did use separate > vector allocations. Looks like that was 7878cba9f0037f5599004b03a1260b32d9050360 - If I follow your commit message your primary goal was to back the bip vecs by a per bio set mempool? I didn't break that (excepting the issue Vivek noted) - but it is true that my patch adds another allocation (when nr_vecs > BIP_INLINE_VECS, anyways). I don't know how big of a deal you think that extra allocation is. If you're against it, this patch isn't really necessary for the immutable bvecs I'm working on - just need it if we want integrity bvecs to be shared like regular bvecs will be. Something else I noticed is bio_integrity_add_page() doesn't merge bvecs when possible, like the regular bio_add_page(). If changing it to merge bvecs wouldn't break anything, then probably most integrity bvecs would be under BIP_INLINE_VECS. Thoughts? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [dm-devel] [PATCH v2 01/26] block: Convert integrity to bvec_alloc_bs(), and a bugfix
On Wed, Sep 12, 2012 at 03:39:18PM -0400, Martin K. Petersen wrote: Kent == Kent Overstreet koverstr...@google.com writes: Kent, Kent To fix the bug first, I'd have to reorder struct bio_pair and then Kent just delete two lines of code from bio_integrity_split(). But the Kent reordering is unnecessary with the refactoring. Well, a bug is a bug and the fix needs to go into stable. So we will need a patch that does not depend on your changes. Alright, good point. I don't have a problem with adding a pointer so clones can point to the parent's vector. But embedding the vector into the bip was a feature. If you check the git log you'll see that originally I did use separate vector allocations. Looks like that was 7878cba9f0037f5599004b03a1260b32d9050360 - If I follow your commit message your primary goal was to back the bip vecs by a per bio set mempool? I didn't break that (excepting the issue Vivek noted) - but it is true that my patch adds another allocation (when nr_vecs BIP_INLINE_VECS, anyways). I don't know how big of a deal you think that extra allocation is. If you're against it, this patch isn't really necessary for the immutable bvecs I'm working on - just need it if we want integrity bvecs to be shared like regular bvecs will be. Something else I noticed is bio_integrity_add_page() doesn't merge bvecs when possible, like the regular bio_add_page(). If changing it to merge bvecs wouldn't break anything, then probably most integrity bvecs would be under BIP_INLINE_VECS. Thoughts? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [dm-devel] [PATCH v2 01/26] block: Convert integrity to bvec_alloc_bs(), and a bugfix
> "Kent" == Kent Overstreet writes: Kent, Kent> To fix the bug first, I'd have to reorder struct bio_pair and then Kent> just delete two lines of code from bio_integrity_split(). But the Kent> reordering is unnecessary with the refactoring. Well, a bug is a bug and the fix needs to go into stable. So we will need a patch that does not depend on your changes. I don't have a problem with adding a pointer so clones can point to the parent's vector. But embedding the vector into the bip was a feature. If you check the git log you'll see that originally I did use separate vector allocations. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [dm-devel] [PATCH v2 01/26] block: Convert integrity to bvec_alloc_bs(), and a bugfix
Kent == Kent Overstreet koverstr...@google.com writes: Kent, Kent To fix the bug first, I'd have to reorder struct bio_pair and then Kent just delete two lines of code from bio_integrity_split(). But the Kent reordering is unnecessary with the refactoring. Well, a bug is a bug and the fix needs to go into stable. So we will need a patch that does not depend on your changes. I don't have a problem with adding a pointer so clones can point to the parent's vector. But embedding the vector into the bip was a feature. If you check the git log you'll see that originally I did use separate vector allocations. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [dm-devel] [PATCH v2 01/26] block: Convert integrity to bvec_alloc_bs(), and a bugfix
On Tue, Sep 11, 2012 at 04:36:43PM -0400, Vivek Goyal wrote: > Also there seems to be too much happening in this patch. Please break > it down in 2. First fix the bio integrity bug you mentioned then > introduce your changes on top. Oh, I forgot - the reason I squashed them into one patch is fixing the bug came for free with the rest of the refactoring, and combining them touches less code. To fix the bug first, I'd have to reorder struct bio_pair and then just delete two lines of code from bio_integrity_split(). But the reordering is unnecessary with the refactoring. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [dm-devel] [PATCH v2 01/26] block: Convert integrity to bvec_alloc_bs(), and a bugfix
On Tue, Sep 11, 2012 at 04:36:43PM -0400, Vivek Goyal wrote:
> On Mon, Sep 10, 2012 at 05:22:12PM -0700, Kent Overstreet wrote:
> > This adds a pointer to the bvec array to struct bio_integrity_payload,
> > instead of the bvecs always being inline; then the bvecs are allocated
> > with bvec_alloc_bs().
>
> If you starting allocating bvec from same mempool for bio and bip,
> are you not breaking the principle of multiple allocations from a
> mempool and hence increasing the possibility of deadlock?
Argh, you're right. I should've thought about that.
It might be nice if mempools had some kind of internal watermark... here
we know we're only going to do at most 2 allocations from the same
mempool, so if we just passed the number we already had allocated to
mempool_alloc(), it could do the right thing and ensure we never
deadlocked.
I _think_ that'd let us make more efficient use of the reserve. But, for
just this it's not really worth the extra code, I think I'll just have
to add another mempool.
> Also there seems to be too much happening in this patch. Please break
> it down in 2. First fix the bio integrity bug you mentioned then
> introduce your changes on top.
Ok, I can do that.
> Thanks
> Vivek
>
> >
> > This is needed eventually for immutable bio vecs - immutable bvecs
> > aren't useful if we still have to copy them, hence the need for the
> > pointer. Less code is always nice too, though.
> >
> > Also fix an amusing bug in bio_integrity_split() - struct bio_pair
> > doesn't have the integrity bvecs after the bio_integrity_payloads, so
> > there was a buffer overrun. The code was confusing pointers with arrays.
> >
> > Signed-off-by: Kent Overstreet
> > CC: Jens Axboe
> > CC: Martin K. Petersen
> > ---
> > fs/bio-integrity.c | 124
> > +---
> > include/linux/bio.h | 5 ++-
> > 2 files changed, 43 insertions(+), 86 deletions(-)
> >
> > diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
> > index a3f28f3..1d64f7f 100644
> > --- a/fs/bio-integrity.c
> > +++ b/fs/bio-integrity.c
> > @@ -27,48 +27,11 @@
> > #include
> > #include
> >
> > -struct integrity_slab {
> > - struct kmem_cache *slab;
> > - unsigned short nr_vecs;
> > - char name[8];
> > -};
> > -
> > -#define IS(x) { .nr_vecs = x, .name = "bip-"__stringify(x) }
> > -struct integrity_slab bip_slab[BIOVEC_NR_POOLS] __read_mostly = {
> > - IS(1), IS(4), IS(16), IS(64), IS(128), IS(BIO_MAX_PAGES),
> > -};
> > -#undef IS
> > +#define BIP_INLINE_VECS4
> >
> > +static struct kmem_cache *bip_slab;
> > static struct workqueue_struct *kintegrityd_wq;
> >
> > -static inline unsigned int vecs_to_idx(unsigned int nr)
> > -{
> > - switch (nr) {
> > - case 1:
> > - return 0;
> > - case 2 ... 4:
> > - return 1;
> > - case 5 ... 16:
> > - return 2;
> > - case 17 ... 64:
> > - return 3;
> > - case 65 ... 128:
> > - return 4;
> > - case 129 ... BIO_MAX_PAGES:
> > - return 5;
> > - default:
> > - BUG();
> > - }
> > -}
> > -
> > -static inline int use_bip_pool(unsigned int idx)
> > -{
> > - if (idx == BIOVEC_MAX_IDX)
> > - return 1;
> > -
> > - return 0;
> > -}
> > -
> > /**
> > * bio_integrity_alloc - Allocate integrity payload and attach it to bio
> > * @bio: bio to attach integrity metadata to
> > @@ -84,37 +47,38 @@ struct bio_integrity_payload
> > *bio_integrity_alloc(struct bio *bio,
> > unsigned int nr_vecs)
> > {
> > struct bio_integrity_payload *bip;
> > - unsigned int idx = vecs_to_idx(nr_vecs);
> > struct bio_set *bs = bio->bi_pool;
> > + unsigned long idx = BIO_POOL_NONE;
> > + unsigned inline_vecs;
> > +
> > + if (!bs) {
> > + 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);
> > + inline_vecs = BIP_INLINE_VECS;
> > + }
> >
> > - if (!bs)
> > - bs = fs_bio_set;
> > -
> > - BUG_ON(bio == NULL);
> > - bip = NULL;
> > + if (unlikely(!bip))
> > + return NULL;
> >
> > - /* Lower order allocations come straight from slab */
> > - if (!use_bip_pool(idx))
> > - bip = kmem_cache_alloc(bip_slab[idx].slab, gfp_mask);
> > + memset(bip, 0, sizeof(struct bio_integrity_payload));
> >
> > - /* Use mempool if lower order alloc failed or max vecs were requested */
> > - if (bip == NULL) {
> > - idx = BIOVEC_MAX_IDX; /* so we free the payload properly later
> > */
> > - bip = mempool_alloc(bs->bio_integrity_pool, gfp_mask);
> > -
> > - if (unlikely(bip == NULL)) {
> > - printk(KERN_ERR "%s: could not alloc bip\n", __func__);
> > - return NULL;
> > - }
> > +
Re: [dm-devel] [PATCH v2 01/26] block: Convert integrity to bvec_alloc_bs(), and a bugfix
On Mon, Sep 10, 2012 at 05:22:12PM -0700, Kent Overstreet wrote:
> This adds a pointer to the bvec array to struct bio_integrity_payload,
> instead of the bvecs always being inline; then the bvecs are allocated
> with bvec_alloc_bs().
If you starting allocating bvec from same mempool for bio and bip,
are you not breaking the principle of multiple allocations from a
mempool and hence increasing the possibility of deadlock?
Also there seems to be too much happening in this patch. Please break
it down in 2. First fix the bio integrity bug you mentioned then
introduce your changes on top.
Thanks
Vivek
>
> This is needed eventually for immutable bio vecs - immutable bvecs
> aren't useful if we still have to copy them, hence the need for the
> pointer. Less code is always nice too, though.
>
> Also fix an amusing bug in bio_integrity_split() - struct bio_pair
> doesn't have the integrity bvecs after the bio_integrity_payloads, so
> there was a buffer overrun. The code was confusing pointers with arrays.
>
> Signed-off-by: Kent Overstreet
> CC: Jens Axboe
> CC: Martin K. Petersen
> ---
> fs/bio-integrity.c | 124
> +---
> include/linux/bio.h | 5 ++-
> 2 files changed, 43 insertions(+), 86 deletions(-)
>
> diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
> index a3f28f3..1d64f7f 100644
> --- a/fs/bio-integrity.c
> +++ b/fs/bio-integrity.c
> @@ -27,48 +27,11 @@
> #include
> #include
>
> -struct integrity_slab {
> - struct kmem_cache *slab;
> - unsigned short nr_vecs;
> - char name[8];
> -};
> -
> -#define IS(x) { .nr_vecs = x, .name = "bip-"__stringify(x) }
> -struct integrity_slab bip_slab[BIOVEC_NR_POOLS] __read_mostly = {
> - IS(1), IS(4), IS(16), IS(64), IS(128), IS(BIO_MAX_PAGES),
> -};
> -#undef IS
> +#define BIP_INLINE_VECS 4
>
> +static struct kmem_cache *bip_slab;
> static struct workqueue_struct *kintegrityd_wq;
>
> -static inline unsigned int vecs_to_idx(unsigned int nr)
> -{
> - switch (nr) {
> - case 1:
> - return 0;
> - case 2 ... 4:
> - return 1;
> - case 5 ... 16:
> - return 2;
> - case 17 ... 64:
> - return 3;
> - case 65 ... 128:
> - return 4;
> - case 129 ... BIO_MAX_PAGES:
> - return 5;
> - default:
> - BUG();
> - }
> -}
> -
> -static inline int use_bip_pool(unsigned int idx)
> -{
> - if (idx == BIOVEC_MAX_IDX)
> - return 1;
> -
> - return 0;
> -}
> -
> /**
> * bio_integrity_alloc - Allocate integrity payload and attach it to bio
> * @bio: bio to attach integrity metadata to
> @@ -84,37 +47,38 @@ struct bio_integrity_payload *bio_integrity_alloc(struct
> bio *bio,
> unsigned int nr_vecs)
> {
> struct bio_integrity_payload *bip;
> - unsigned int idx = vecs_to_idx(nr_vecs);
> struct bio_set *bs = bio->bi_pool;
> + unsigned long idx = BIO_POOL_NONE;
> + unsigned inline_vecs;
> +
> + if (!bs) {
> + 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);
> + inline_vecs = BIP_INLINE_VECS;
> + }
>
> - if (!bs)
> - bs = fs_bio_set;
> -
> - BUG_ON(bio == NULL);
> - bip = NULL;
> + if (unlikely(!bip))
> + return NULL;
>
> - /* Lower order allocations come straight from slab */
> - if (!use_bip_pool(idx))
> - bip = kmem_cache_alloc(bip_slab[idx].slab, gfp_mask);
> + memset(bip, 0, sizeof(struct bio_integrity_payload));
>
> - /* Use mempool if lower order alloc failed or max vecs were requested */
> - if (bip == NULL) {
> - idx = BIOVEC_MAX_IDX; /* so we free the payload properly later
> */
> - bip = mempool_alloc(bs->bio_integrity_pool, gfp_mask);
> -
> - if (unlikely(bip == NULL)) {
> - printk(KERN_ERR "%s: could not alloc bip\n", __func__);
> - return NULL;
> - }
> + if (nr_vecs > inline_vecs) {
> + bip->bip_vec = bvec_alloc_bs(gfp_mask, nr_vecs, , bs);
> + if (!bip->bip_vec)
> + goto err;
> }
>
> - memset(bip, 0, sizeof(*bip));
> -
> bip->bip_slab = idx;
> bip->bip_bio = bio;
> bio->bi_integrity = bip;
>
> return bip;
> +err:
> + mempool_free(bip, bs->bio_integrity_pool);
> + return NULL;
> }
> EXPORT_SYMBOL(bio_integrity_alloc);
>
> @@ -130,20 +94,19 @@ void bio_integrity_free(struct bio *bio)
> struct bio_integrity_payload *bip = bio->bi_integrity;
> struct bio_set *bs = bio->bi_pool;
>
> - if (!bs)
> - bs = fs_bio_set;
> -
> - BUG_ON(bip == NULL);
> -
>
Re: [dm-devel] [PATCH v2 01/26] block: Convert integrity to bvec_alloc_bs(), and a bugfix
On Mon, Sep 10, 2012 at 05:22:12PM -0700, Kent Overstreet wrote:
This adds a pointer to the bvec array to struct bio_integrity_payload,
instead of the bvecs always being inline; then the bvecs are allocated
with bvec_alloc_bs().
If you starting allocating bvec from same mempool for bio and bip,
are you not breaking the principle of multiple allocations from a
mempool and hence increasing the possibility of deadlock?
Also there seems to be too much happening in this patch. Please break
it down in 2. First fix the bio integrity bug you mentioned then
introduce your changes on top.
Thanks
Vivek
This is needed eventually for immutable bio vecs - immutable bvecs
aren't useful if we still have to copy them, hence the need for the
pointer. Less code is always nice too, though.
Also fix an amusing bug in bio_integrity_split() - struct bio_pair
doesn't have the integrity bvecs after the bio_integrity_payloads, so
there was a buffer overrun. The code was confusing pointers with arrays.
Signed-off-by: Kent Overstreet koverstr...@google.com
CC: Jens Axboe ax...@kernel.dk
CC: Martin K. Petersen martin.peter...@oracle.com
---
fs/bio-integrity.c | 124
+---
include/linux/bio.h | 5 ++-
2 files changed, 43 insertions(+), 86 deletions(-)
diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
index a3f28f3..1d64f7f 100644
--- a/fs/bio-integrity.c
+++ b/fs/bio-integrity.c
@@ -27,48 +27,11 @@
#include linux/workqueue.h
#include linux/slab.h
-struct integrity_slab {
- struct kmem_cache *slab;
- unsigned short nr_vecs;
- char name[8];
-};
-
-#define IS(x) { .nr_vecs = x, .name = bip-__stringify(x) }
-struct integrity_slab bip_slab[BIOVEC_NR_POOLS] __read_mostly = {
- IS(1), IS(4), IS(16), IS(64), IS(128), IS(BIO_MAX_PAGES),
-};
-#undef IS
+#define BIP_INLINE_VECS 4
+static struct kmem_cache *bip_slab;
static struct workqueue_struct *kintegrityd_wq;
-static inline unsigned int vecs_to_idx(unsigned int nr)
-{
- switch (nr) {
- case 1:
- return 0;
- case 2 ... 4:
- return 1;
- case 5 ... 16:
- return 2;
- case 17 ... 64:
- return 3;
- case 65 ... 128:
- return 4;
- case 129 ... BIO_MAX_PAGES:
- return 5;
- default:
- BUG();
- }
-}
-
-static inline int use_bip_pool(unsigned int idx)
-{
- if (idx == BIOVEC_MAX_IDX)
- return 1;
-
- return 0;
-}
-
/**
* bio_integrity_alloc - Allocate integrity payload and attach it to bio
* @bio: bio to attach integrity metadata to
@@ -84,37 +47,38 @@ struct bio_integrity_payload *bio_integrity_alloc(struct
bio *bio,
unsigned int nr_vecs)
{
struct bio_integrity_payload *bip;
- unsigned int idx = vecs_to_idx(nr_vecs);
struct bio_set *bs = bio-bi_pool;
+ unsigned long idx = BIO_POOL_NONE;
+ unsigned inline_vecs;
+
+ if (!bs) {
+ 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);
+ inline_vecs = BIP_INLINE_VECS;
+ }
- if (!bs)
- bs = fs_bio_set;
-
- BUG_ON(bio == NULL);
- bip = NULL;
+ if (unlikely(!bip))
+ return NULL;
- /* Lower order allocations come straight from slab */
- if (!use_bip_pool(idx))
- bip = kmem_cache_alloc(bip_slab[idx].slab, gfp_mask);
+ memset(bip, 0, sizeof(struct bio_integrity_payload));
- /* Use mempool if lower order alloc failed or max vecs were requested */
- if (bip == NULL) {
- idx = BIOVEC_MAX_IDX; /* so we free the payload properly later
*/
- bip = mempool_alloc(bs-bio_integrity_pool, gfp_mask);
-
- if (unlikely(bip == NULL)) {
- printk(KERN_ERR %s: could not alloc bip\n, __func__);
- return NULL;
- }
+ if (nr_vecs inline_vecs) {
+ bip-bip_vec = bvec_alloc_bs(gfp_mask, nr_vecs, idx, bs);
+ if (!bip-bip_vec)
+ goto err;
}
- memset(bip, 0, sizeof(*bip));
-
bip-bip_slab = idx;
bip-bip_bio = bio;
bio-bi_integrity = bip;
return bip;
+err:
+ mempool_free(bip, bs-bio_integrity_pool);
+ return NULL;
}
EXPORT_SYMBOL(bio_integrity_alloc);
@@ -130,20 +94,19 @@ void bio_integrity_free(struct bio *bio)
struct bio_integrity_payload *bip = bio-bi_integrity;
struct bio_set *bs = bio-bi_pool;
- if (!bs)
- bs = fs_bio_set;
-
- BUG_ON(bip == NULL);
-
/* A cloned bio doesn't own the integrity metadata */
if
Re: [dm-devel] [PATCH v2 01/26] block: Convert integrity to bvec_alloc_bs(), and a bugfix
On Tue, Sep 11, 2012 at 04:36:43PM -0400, Vivek Goyal wrote:
On Mon, Sep 10, 2012 at 05:22:12PM -0700, Kent Overstreet wrote:
This adds a pointer to the bvec array to struct bio_integrity_payload,
instead of the bvecs always being inline; then the bvecs are allocated
with bvec_alloc_bs().
If you starting allocating bvec from same mempool for bio and bip,
are you not breaking the principle of multiple allocations from a
mempool and hence increasing the possibility of deadlock?
Argh, you're right. I should've thought about that.
It might be nice if mempools had some kind of internal watermark... here
we know we're only going to do at most 2 allocations from the same
mempool, so if we just passed the number we already had allocated to
mempool_alloc(), it could do the right thing and ensure we never
deadlocked.
I _think_ that'd let us make more efficient use of the reserve. But, for
just this it's not really worth the extra code, I think I'll just have
to add another mempool.
Also there seems to be too much happening in this patch. Please break
it down in 2. First fix the bio integrity bug you mentioned then
introduce your changes on top.
Ok, I can do that.
Thanks
Vivek
This is needed eventually for immutable bio vecs - immutable bvecs
aren't useful if we still have to copy them, hence the need for the
pointer. Less code is always nice too, though.
Also fix an amusing bug in bio_integrity_split() - struct bio_pair
doesn't have the integrity bvecs after the bio_integrity_payloads, so
there was a buffer overrun. The code was confusing pointers with arrays.
Signed-off-by: Kent Overstreet koverstr...@google.com
CC: Jens Axboe ax...@kernel.dk
CC: Martin K. Petersen martin.peter...@oracle.com
---
fs/bio-integrity.c | 124
+---
include/linux/bio.h | 5 ++-
2 files changed, 43 insertions(+), 86 deletions(-)
diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
index a3f28f3..1d64f7f 100644
--- a/fs/bio-integrity.c
+++ b/fs/bio-integrity.c
@@ -27,48 +27,11 @@
#include linux/workqueue.h
#include linux/slab.h
-struct integrity_slab {
- struct kmem_cache *slab;
- unsigned short nr_vecs;
- char name[8];
-};
-
-#define IS(x) { .nr_vecs = x, .name = bip-__stringify(x) }
-struct integrity_slab bip_slab[BIOVEC_NR_POOLS] __read_mostly = {
- IS(1), IS(4), IS(16), IS(64), IS(128), IS(BIO_MAX_PAGES),
-};
-#undef IS
+#define BIP_INLINE_VECS4
+static struct kmem_cache *bip_slab;
static struct workqueue_struct *kintegrityd_wq;
-static inline unsigned int vecs_to_idx(unsigned int nr)
-{
- switch (nr) {
- case 1:
- return 0;
- case 2 ... 4:
- return 1;
- case 5 ... 16:
- return 2;
- case 17 ... 64:
- return 3;
- case 65 ... 128:
- return 4;
- case 129 ... BIO_MAX_PAGES:
- return 5;
- default:
- BUG();
- }
-}
-
-static inline int use_bip_pool(unsigned int idx)
-{
- if (idx == BIOVEC_MAX_IDX)
- return 1;
-
- return 0;
-}
-
/**
* bio_integrity_alloc - Allocate integrity payload and attach it to bio
* @bio: bio to attach integrity metadata to
@@ -84,37 +47,38 @@ struct bio_integrity_payload
*bio_integrity_alloc(struct bio *bio,
unsigned int nr_vecs)
{
struct bio_integrity_payload *bip;
- unsigned int idx = vecs_to_idx(nr_vecs);
struct bio_set *bs = bio-bi_pool;
+ unsigned long idx = BIO_POOL_NONE;
+ unsigned inline_vecs;
+
+ if (!bs) {
+ 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);
+ inline_vecs = BIP_INLINE_VECS;
+ }
- if (!bs)
- bs = fs_bio_set;
-
- BUG_ON(bio == NULL);
- bip = NULL;
+ if (unlikely(!bip))
+ return NULL;
- /* Lower order allocations come straight from slab */
- if (!use_bip_pool(idx))
- bip = kmem_cache_alloc(bip_slab[idx].slab, gfp_mask);
+ memset(bip, 0, sizeof(struct bio_integrity_payload));
- /* Use mempool if lower order alloc failed or max vecs were requested */
- if (bip == NULL) {
- idx = BIOVEC_MAX_IDX; /* so we free the payload properly later
*/
- bip = mempool_alloc(bs-bio_integrity_pool, gfp_mask);
-
- if (unlikely(bip == NULL)) {
- printk(KERN_ERR %s: could not alloc bip\n, __func__);
- return NULL;
- }
+ if (nr_vecs inline_vecs) {
+ bip-bip_vec = bvec_alloc_bs(gfp_mask, nr_vecs, idx, bs);
+ if (!bip-bip_vec)
+ goto err;
Re: [dm-devel] [PATCH v2 01/26] block: Convert integrity to bvec_alloc_bs(), and a bugfix
On Tue, Sep 11, 2012 at 04:36:43PM -0400, Vivek Goyal wrote: Also there seems to be too much happening in this patch. Please break it down in 2. First fix the bio integrity bug you mentioned then introduce your changes on top. Oh, I forgot - the reason I squashed them into one patch is fixing the bug came for free with the rest of the refactoring, and combining them touches less code. To fix the bug first, I'd have to reorder struct bio_pair and then just delete two lines of code from bio_integrity_split(). But the reordering is unnecessary with the refactoring. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
