Re: Resend: [PATCH] blkdev: fix blkdev_issue_zeroout return value

Dmitry Monakhov
2010-08-06T11:15:55Z

[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]

Jens Axboe <axboe@kernel.dk> writes:

> On 2010-08-06 12:42, Dmitry Monakhov wrote:
>> Hi Jens,
>> Seems that my first mail was missed somewhere.
>> I've found couple of trivial issues in blkdev_issue_zeroout()
>> implementation. Unfortunately I've miss during initial testing phase
>> because always called it with BARRIER|WAIT flags.
>
> BTW, this:
>
> @@ -218,15 +222,18 @@ submit:
> /* One of bios in the batch was completed with error.*/
> ret = -EIO;
>
> - if (ret)
> + if (ret && ret != -ENOMEM)
> goto out;
>
> if (test_bit(BIO_EOPNOTSUPP, &bb.flags)) {
> ret = -EOPNOTSUPP;
> goto out;
> }
> - if (nr_sects != 0)
> + if (nr_sects != 0) {
> + if (ret == -ENOMEM)
> + io_schedule();
> goto submit;
> + }
> out:
> return ret;
> }
>
> is broken. Either the caller sets __GFP_WAIT and then bio_alloc() will
> not fail, or GFP_ATOMIC is used knowing that the call can fail and
> return ENOMEM. Don't code in retry logic like this.
Ok, my fault and in fact i've done in explicitly. I just thought
that blk-layer is some times an exception from general GFP_ATOMIC rule
because in some places in blk-layer we stick to GFP_NOFAIL semantics
regardless to actual gfp flags.

New version attached.

[-- Attachment #2: 0001-blkdev-fix-blkdev_issue_zeroout-return-value.patch --]
[-- Type: text/plain, Size: 1345 bytes --]

>From 32236cd453f070807a91631a2de12478f872edd3 Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Fri, 6 Aug 2010 15:06:01 +0400
Subject: [PATCH] blkdev: fix blkdev_issue_zeroout return value

- If function called without barrier option retvalue is incorrect

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
block/blk-lib.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 5d793e1..c1fc55a 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -145,7 +145,7 @@ static void bio_batch_end_io(struct bio *bio, int err)
int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask, unsigned long flags)
{
- int ret = 0;
+ int ret;
struct bio *bio;
struct bio_batch bb;
unsigned int sz, issued = 0;
@@ -163,11 +163,14 @@ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
return ret;
}
submit:
+ ret = 0;
while (nr_sects != 0) {
bio = bio_alloc(gfp_mask,
min(nr_sects, (sector_t)BIO_MAX_PAGES));
- if (!bio)
+ if (!bio) {
+ ret = -ENOMEM;
break;
+ }

bio->bi_sector = sector;
bio->bi_bdev = bdev;
@@ -186,6 +189,7 @@ submit:
if (ret < (sz << 9))
break;
}
+ ret = 0;
issued++;
submit_bio(WRITE, bio);
}
--
1.6.6.1