Re: [PATCH V2] block: correctly fallback for zeroout

On Tue, Jun 07, 2016 at 07:58:25AM -0700, Shaohua Li wrote: > > I didn't follow. io_err is only and always set when ret == 0. io_err is > meanless if ret != 0, because that means the disk doesn't support discard and > we don't dispatch discard IO. why should we initialized io_err to 0? My mistake

Re: [PATCH V2] block: correctly fallback for zeroout

On Tue, Jun 07, 2016 at 07:58:25AM -0700, Shaohua Li wrote: > > I didn't follow. io_err is only and always set when ret == 0. io_err is > meanless if ret != 0, because that means the disk doesn't support discard and > we don't dispatch discard IO. why should we initialized io_err to 0? My mistake

Re: [PATCH V2] block: correctly fallback for zeroout

On Tue, Jun 14, 2016 at 10:14:50PM -0400, Martin K. Petersen wrote: > > "Christoph" == Christoph Hellwig writes: > > Christoph> And I'd much prefer to get this right now. It's not like > Christoph> this is recently introduced behavior. > > Unfortunately there are quite

Re: [PATCH V2] block: correctly fallback for zeroout

On Tue, Jun 14, 2016 at 10:14:50PM -0400, Martin K. Petersen wrote: > > "Christoph" == Christoph Hellwig writes: > > Christoph> And I'd much prefer to get this right now. It's not like > Christoph> this is recently introduced behavior. > > Unfortunately there are quite a few callers of

Re: [PATCH V2] block: correctly fallback for zeroout

On Tue, Jun 14 2016 at 10:30pm -0400, Martin K. Petersen wrote: > > "Mike" == Mike Snitzer writes: > > Mike, > > Mike> so long story short: making this change to remove this so-called > Mike> "stupid behaviour" will require code like > Mike>

Re: [PATCH V2] block: correctly fallback for zeroout

On Tue, Jun 14 2016 at 10:30pm -0400, Martin K. Petersen wrote: > > "Mike" == Mike Snitzer writes: > > Mike, > > Mike> so long story short: making this change to remove this so-called > Mike> "stupid behaviour" will require code like > Mike> drivers/md/dm-thin.c:issue_discard(() to check

Re: [PATCH V2] block: correctly fallback for zeroout

> "Mike" == Mike Snitzer writes: Mike, Mike> so long story short: making this change to remove this so-called Mike> "stupid behaviour" will require code like Mike> drivers/md/dm-thin.c:issue_discard(() to check the return from Mike> __blkdev_issue_discard() and if it is

Re: [PATCH V2] block: correctly fallback for zeroout

> "Mike" == Mike Snitzer writes: Mike, Mike> so long story short: making this change to remove this so-called Mike> "stupid behaviour" will require code like Mike> drivers/md/dm-thin.c:issue_discard(() to check the return from Mike> __blkdev_issue_discard() and if it is -EOPNOTSUPP then it

Re: [PATCH V2] block: correctly fallback for zeroout

> "Christoph" == Christoph Hellwig writes: Christoph> We can move the sanity checks out. Or even better get rid of Christoph> the stupid behavior of ignoring the late -EOPNOTSUPP in this Christoph> low level helper and instead leaving it to the caller(s) that Christoph>

Re: [PATCH V2] block: correctly fallback for zeroout

> "Christoph" == Christoph Hellwig writes: Christoph> We can move the sanity checks out. Or even better get rid of Christoph> the stupid behavior of ignoring the late -EOPNOTSUPP in this Christoph> low level helper and instead leaving it to the caller(s) that Christoph> care. It definitely

Re: [PATCH V2] block: correctly fallback for zeroout

On Mon, Jun 13 2016 at 4:20am -0400, Christoph Hellwig wrote: > On Fri, Jun 10, 2016 at 09:49:44PM -0400, Martin K. Petersen wrote: > > >> What does the extra io_err buy us? Just have this function return an > > >> error. And then in blkdev_issue_discard if you get

Re: [PATCH V2] block: correctly fallback for zeroout

On Mon, Jun 13 2016 at 4:20am -0400, Christoph Hellwig wrote: > On Fri, Jun 10, 2016 at 09:49:44PM -0400, Martin K. Petersen wrote: > > >> What does the extra io_err buy us? Just have this function return an > > >> error. And then in blkdev_issue_discard if you get -EOPNOTSUPP you > > >>

Re: [PATCH V2] block: correctly fallback for zeroout

On Fri, Jun 10, 2016 at 09:49:44PM -0400, Martin K. Petersen wrote: > >> What does the extra io_err buy us? Just have this function return an > >> error. And then in blkdev_issue_discard if you get -EOPNOTSUPP you > >> special case it there. > > Shaohua> The __blkdev_issue_discard returns

Re: [PATCH V2] block: correctly fallback for zeroout

On Fri, Jun 10, 2016 at 09:49:44PM -0400, Martin K. Petersen wrote: > >> What does the extra io_err buy us? Just have this function return an > >> error. And then in blkdev_issue_discard if you get -EOPNOTSUPP you > >> special case it there. > > Shaohua> The __blkdev_issue_discard returns

Re: [PATCH V2] block: correctly fallback for zeroout

> "Shaohua" == Shaohua Li writes: Shaohua, >> What does the extra io_err buy us? Just have this function return an >> error. And then in blkdev_issue_discard if you get -EOPNOTSUPP you >> special case it there. Shaohua> The __blkdev_issue_discard returns -EOPNOTSUPP if disk

Re: [PATCH V2] block: correctly fallback for zeroout

> "Shaohua" == Shaohua Li writes: Shaohua, >> What does the extra io_err buy us? Just have this function return an >> error. And then in blkdev_issue_discard if you get -EOPNOTSUPP you >> special case it there. Shaohua> The __blkdev_issue_discard returns -EOPNOTSUPP if disk doesn't

Re: [PATCH V2] block: correctly fallback for zeroout

On Thu, Jun 09, 2016 at 10:04:08PM -0400, Martin K. Petersen wrote: > > "Shaohua" == Shaohua Li writes: > > Shaohua, > > diff --git a/block/blk-lib.c b/block/blk-lib.c > index 23d7f30..a3a26c8 100644 > --- a/block/blk-lib.c > +++ b/block/blk-lib.c > @@ -84,6 +84,28 @@ int

Re: [PATCH V2] block: correctly fallback for zeroout

On Thu, Jun 09, 2016 at 10:04:08PM -0400, Martin K. Petersen wrote: > > "Shaohua" == Shaohua Li writes: > > Shaohua, > > diff --git a/block/blk-lib.c b/block/blk-lib.c > index 23d7f30..a3a26c8 100644 > --- a/block/blk-lib.c > +++ b/block/blk-lib.c > @@ -84,6 +84,28 @@ int

Re: [PATCH V2] block: correctly fallback for zeroout

> "Shaohua" == Shaohua Li writes: Shaohua, diff --git a/block/blk-lib.c b/block/blk-lib.c index 23d7f30..a3a26c8 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -84,6 +84,28 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector, }

Re: [PATCH V2] block: correctly fallback for zeroout

> "Shaohua" == Shaohua Li writes: Shaohua, diff --git a/block/blk-lib.c b/block/blk-lib.c index 23d7f30..a3a26c8 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -84,6 +84,28 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector, }

Re: [PATCH V2] block: correctly fallback for zeroout

On Tue, Jun 07, 2016 at 05:50:49AM +0100, Sitsofe Wheeler wrote: > On Mon, Jun 06, 2016 at 03:33:58PM -0700, Shaohua Li wrote: > > blkdev_issue_zeroout try discard/writesame first, if they fail, zeroout > > fallback to regular write. The problem is discard/writesame doesn't > > return error for

Re: [PATCH V2] block: correctly fallback for zeroout

On Tue, Jun 07, 2016 at 05:50:49AM +0100, Sitsofe Wheeler wrote: > On Mon, Jun 06, 2016 at 03:33:58PM -0700, Shaohua Li wrote: > > blkdev_issue_zeroout try discard/writesame first, if they fail, zeroout > > fallback to regular write. The problem is discard/writesame doesn't > > return error for

Re: [PATCH V2] block: correctly fallback for zeroout

On Mon, Jun 06, 2016 at 03:33:58PM -0700, Shaohua Li wrote: > blkdev_issue_zeroout try discard/writesame first, if they fail, zeroout > fallback to regular write. The problem is discard/writesame doesn't > return error for -EOPNOTSUPP, then zeroout can't do fallback and leave > disk data not

Re: [PATCH V2] block: correctly fallback for zeroout

On Mon, Jun 06, 2016 at 03:33:58PM -0700, Shaohua Li wrote: > blkdev_issue_zeroout try discard/writesame first, if they fail, zeroout > fallback to regular write. The problem is discard/writesame doesn't > return error for -EOPNOTSUPP, then zeroout can't do fallback and leave > disk data not

[PATCH V2] block: correctly fallback for zeroout

blkdev_issue_zeroout try discard/writesame first, if they fail, zeroout fallback to regular write. The problem is discard/writesame doesn't return error for -EOPNOTSUPP, then zeroout can't do fallback and leave disk data not changed. zeroout should have guaranteed zero-fill behavior.

[PATCH V2] block: correctly fallback for zeroout

blkdev_issue_zeroout try discard/writesame first, if they fail, zeroout fallback to regular write. The problem is discard/writesame doesn't return error for -EOPNOTSUPP, then zeroout can't do fallback and leave disk data not changed. zeroout should have guaranteed zero-fill behavior.