lkml.org 
[lkml]   [2009]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4/5] bio: fix bio_kmalloc()
    Date
    Impact: fix bio_kmalloc() and its destruction path

    bio_kmalloc() was broken in two ways.

    * bvec_alloc_bs() first allocates bvec using kmalloc() and then
    ignores it and allocates again like non-kmalloc bvecs.

    * bio_kmalloc_destructor() didn't check for and free bio integrity
    data.

    This patch fixes the above problems. kmalloc patch is separated out
    from bio_alloc_bioset() and allocates the requested number of bvecs as
    inline bvecs.

    * bio_alloc_bioset() no longer takes NULL @bs. None other than
    bio_kmalloc() used it and outside users can't know how it was
    allocated anyway.

    * Define and use BIO_POOL_NONE so that pool index check in
    bvec_free_bs() triggers if inline or kmalloc allocated bvec gets
    there.

    * Relocate destructors on top of each allocation function so that how
    they're used is more clear.

    Jens Axboe suggested allocating bvecs inline.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Cc: Jens Axboe <jens.axboe@oracle.com>
    ---
    fs/bio.c | 108 ++++++++++++++++++++++++++-------------------------
    include/linux/bio.h | 1 +
    2 files changed, 56 insertions(+), 53 deletions(-)

    diff --git a/fs/bio.c b/fs/bio.c
    index e0c9e54..6c2aef6 100644
    --- a/fs/bio.c
    +++ b/fs/bio.c
    @@ -175,14 +175,6 @@ struct bio_vec *bvec_alloc_bs(gfp_t gfp_mask, int nr, unsigned long *idx,
    struct bio_vec *bvl;

    /*
    - * If 'bs' is given, lookup the pool and do the mempool alloc.
    - * If not, this is a bio_kmalloc() allocation and just do a
    - * kzalloc() for the exact number of vecs right away.
    - */
    - if (!bs)
    - bvl = kmalloc(nr * sizeof(struct bio_vec), gfp_mask);
    -
    - /*
    * see comment near bvec_array define!
    */
    switch (nr) {
    @@ -260,21 +252,6 @@ void bio_free(struct bio *bio, struct bio_set *bs)
    mempool_free(p, bs->bio_pool);
    }

    -/*
    - * default destructor for a bio allocated with bio_alloc_bioset()
    - */
    -static void bio_fs_destructor(struct bio *bio)
    -{
    - bio_free(bio, fs_bio_set);
    -}
    -
    -static void bio_kmalloc_destructor(struct bio *bio)
    -{
    - if (bio_has_allocated_vec(bio))
    - kfree(bio->bi_io_vec);
    - kfree(bio);
    -}
    -
    void bio_init(struct bio *bio)
    {
    memset(bio, 0, sizeof(*bio));
    @@ -301,21 +278,15 @@ void bio_init(struct bio *bio)
    **/
    struct bio *bio_alloc_bioset(gfp_t gfp_mask, int nr_iovecs, struct bio_set *bs)
    {
    + unsigned long idx = BIO_POOL_NONE;
    struct bio_vec *bvl = NULL;
    - struct bio *bio = NULL;
    - unsigned long idx = 0;
    - void *p = NULL;
    -
    - if (bs) {
    - p = mempool_alloc(bs->bio_pool, gfp_mask);
    - if (!p)
    - goto err;
    - bio = p + bs->front_pad;
    - } else {
    - bio = kmalloc(sizeof(*bio), gfp_mask);
    - if (!bio)
    - goto err;
    - }
    + struct bio *bio;
    + void *p;
    +
    + p = mempool_alloc(bs->bio_pool, gfp_mask);
    + if (unlikely(!p))
    + return NULL;
    + bio = p + bs->front_pad;

    bio_init(bio);

    @@ -332,22 +303,33 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask, int nr_iovecs, struct bio_set *bs)

    nr_iovecs = bvec_nr_vecs(idx);
    }
    +out_set:
    bio->bi_flags |= idx << BIO_POOL_OFFSET;
    bio->bi_max_vecs = nr_iovecs;
    -out_set:
    bio->bi_io_vec = bvl;
    -
    return bio;

    err_free:
    - if (bs)
    - mempool_free(p, bs->bio_pool);
    - else
    - kfree(bio);
    -err:
    + mempool_free(p, bs->bio_pool);
    return NULL;
    }

    +static void bio_fs_destructor(struct bio *bio)
    +{
    + bio_free(bio, fs_bio_set);
    +}
    +
    +/**
    + * bio_alloc - allocate a new bio, memory pool backed
    + * @gfp_mask: allocation mask to use
    + * @nr_iovecs: number of iovecs
    + *
    + * Allocate a new bio with @nr_iovecs bvecs. If @gfp_mask
    + * contains __GFP_WAIT, the allocation is guaranteed to succeed.
    + *
    + * RETURNS:
    + * Pointer to new bio on success, NULL on failure.
    + */
    struct bio *bio_alloc(gfp_t gfp_mask, int nr_iovecs)
    {
    struct bio *bio = bio_alloc_bioset(gfp_mask, nr_iovecs, fs_bio_set);
    @@ -358,19 +340,39 @@ struct bio *bio_alloc(gfp_t gfp_mask, int nr_iovecs)
    return bio;
    }

    -/*
    - * Like bio_alloc(), but doesn't use a mempool backing. This means that
    - * it CAN fail, but while bio_alloc() can only be used for allocations
    - * that have a short (finite) life span, bio_kmalloc() should be used
    - * for more permanent bio allocations (like allocating some bio's for
    - * initalization or setup purposes).
    +static void bio_kmalloc_destructor(struct bio *bio)
    +{
    + if (bio_integrity(bio))
    + bio_integrity_free(bio);
    + kfree(bio);
    +}
    +
    +/**
    + * bio_kmalloc - allocate a new bio
    + * @gfp_mask: allocation mask to use
    + * @nr_iovecs: number of iovecs
    + *
    + * Similar to bio_alloc() but uses regular kmalloc for allocation
    + * and can fail unless __GFP_NOFAIL is set in @gfp_mask. This is
    + * useful for more permanant or over-sized bio allocations.
    + *
    + * RETURNS:
    + * Poitner to new bio on success, NULL on failure.
    */
    struct bio *bio_kmalloc(gfp_t gfp_mask, int nr_iovecs)
    {
    - struct bio *bio = bio_alloc_bioset(gfp_mask, nr_iovecs, NULL);
    + struct bio *bio;

    - if (bio)
    - bio->bi_destructor = bio_kmalloc_destructor;
    + bio = kmalloc(sizeof(struct bio) + nr_iovecs * sizeof(struct bio_vec),
    + gfp_mask);
    + if (unlikely(!bio))
    + return NULL;
    +
    + bio_init(bio);
    + bio->bi_flags |= BIO_POOL_NONE << BIO_POOL_OFFSET;
    + bio->bi_max_vecs = nr_iovecs;
    + bio->bi_io_vec = bio->bi_inline_vecs;
    + bio->bi_destructor = bio_kmalloc_destructor;

    return bio;
    }
    diff --git a/include/linux/bio.h b/include/linux/bio.h
    index b89cf2d..7b214fd 100644
    --- a/include/linux/bio.h
    +++ b/include/linux/bio.h
    @@ -132,6 +132,7 @@ struct bio {
    * top 4 bits of bio flags indicate the pool this bio came from
    */
    #define BIO_POOL_BITS (4)
    +#define BIO_POOL_NONE ((1UL << BIO_POOL_BITS) - 1)
    #define BIO_POOL_OFFSET (BITS_PER_LONG - BIO_POOL_BITS)
    #define BIO_POOL_MASK (1UL << BIO_POOL_OFFSET)
    #define BIO_POOL_IDX(bio) ((bio)->bi_flags >> BIO_POOL_OFFSET)
    --
    1.6.0.2


    \
     
     \ /
      Last update: 2009-04-15 15:15    [W:0.037 / U:118.704 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site