Reviewed-by: Yehuda Sadeh <yehuda@xxxxxxxxxxx>
we still need to fix the bio_pair leak though
On Fri, Aug 24, 2012 at 9:35 AM, Alex Elder <elder@xxxxxxxxxxx> wrote:
> In bio_chain_clone(), at the end of the function the bi_next field
> of the tail of the new bio chain is nulled. This isn't necessary,
> because if "tail" is non-null, its value will be the last bio
> structure allocated at the top of the while loop in that function.
> And before that structure is added to the end of the new chain, its
> bi_next pointer is always made null.
>
> While touching that function, clean a few other things:
> - define each local variable on its own line
> - move the definition of "tmp" to an inner scope
> - move the modification of gfpmask closer to where it's used
> - rearrange the logic that sets the chain's tail pointer
>
> Signed-off-by: Alex Elder <elder@xxxxxxxxxxx>
> ---
> drivers/block/rbd.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index e94336d..bb8dad7 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -741,7 +741,9 @@ static struct bio *bio_chain_clone(struct bio **old,
> struct bio **next,
> struct bio_pair **bp,
> int len, gfp_t gfpmask)
> {
> - struct bio *tmp, *old_chain = *old, *new_chain = NULL, *tail = NULL;
> + struct bio *old_chain = *old;
> + struct bio *new_chain = NULL;
> + struct bio *tail;
> int total = 0;
>
> if (*bp) {
> @@ -750,9 +752,12 @@ static struct bio *bio_chain_clone(struct bio
> **old, struct bio **next,
> }
>
> while (old_chain && (total < len)) {
> + struct bio *tmp;
> +
> tmp = bio_kmalloc(gfpmask, old_chain->bi_max_vecs);
> if (!tmp)
> goto err_out;
> + gfpmask &= ~__GFP_WAIT; /* can't wait after the first */
>
> if (total + old_chain->bi_size > len) {
> struct bio_pair *bp;
> @@ -780,15 +785,12 @@ static struct bio *bio_chain_clone(struct bio
> **old, struct bio **next,
> }
>
> tmp->bi_bdev = NULL;
> - gfpmask &= ~__GFP_WAIT;
> tmp->bi_next = NULL;
> -
> - if (!new_chain) {
> - new_chain = tail = tmp;
> - } else {
> + if (new_chain)
> tail->bi_next = tmp;
> - tail = tmp;
> - }
> + else
> + new_chain = tmp;
> + tail = tmp;
> old_chain = old_chain->bi_next;
>
> total += tmp->bi_size;
> @@ -796,9 +798,6 @@ static struct bio *bio_chain_clone(struct bio **old,
> struct bio **next,
>
> BUG_ON(total < len);
>
> - if (tail)
> - tail->bi_next = NULL;
> -
> *old = old_chain;
>
> return new_chain;
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
