Resend: [PATCH] blkdev: fix blkdev_issue_zeroout return value
in [Kernel]
|
From: Dmitry Monakhov on 6 Aug 2010 06:50 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.
From: Jens Axboe on 6 Aug 2010 07:00 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. > OK, I will add this so it can go out with the next pull request. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Dmitry Monakhov on 6 Aug 2010 07:20 Jens Axboe <axboe(a)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.
From: Jens Axboe on 6 Aug 2010 07:30 On 2010-08-06 13:15, Dmitry Monakhov wrote: > Jens Axboe <axboe(a)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. Thanks that looks better, now really added. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|
Pages: 1 Prev: [GIT PULL] nilfs2 updates for 2.6.36 Next: block/IO bits for 2.6.36-rc1 |