* [PATCH v3 0/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
@ 2017-10-16 13:59 Ilya Dryomov
2017-10-16 13:59 ` [PATCH v3 1/2] block: factor out __blkdev_issue_zero_pages() Ilya Dryomov
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Ilya Dryomov @ 2017-10-16 13:59 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin K. Petersen, Hannes Reinecke, Jens Axboe, linux-block
Hi Christoph, Martin,
blkdev_issue_zeroout() now checks for any error. This required a minor
refactor, so I dropped the stable tag, Jens can add it back if needed.
v2 -> v3:
- another code flow change in blkdev_issue_zeroout() suggested by
Christoph -- no functional changes
v1 -> v2:
- changed code flow in blkdev_issue_zeroout() according to Christoph's
suggestion
- this required adding additional checks to blkdev_issue_zeroout() and
__blkdev_issue_zero_pages(), the latter is no longer void
Previous version at
https://marc.info/?l=linux-block&m=150712940922089&w=2
Thanks,
Ilya
Ilya Dryomov (2):
block: factor out __blkdev_issue_zero_pages()
block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
block/blk-lib.c | 108 +++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 72 insertions(+), 36 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/2] block: factor out __blkdev_issue_zero_pages()
2017-10-16 13:59 [PATCH v3 0/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout() Ilya Dryomov
@ 2017-10-16 13:59 ` Ilya Dryomov
2017-10-18 6:35 ` Christoph Hellwig
2017-10-18 22:40 ` Martin K. Petersen
2017-10-16 13:59 ` [PATCH v3 2/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout() Ilya Dryomov
2017-10-25 6:27 ` [PATCH v3 0/2] " Ilya Dryomov
2 siblings, 2 replies; 9+ messages in thread
From: Ilya Dryomov @ 2017-10-16 13:59 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin K. Petersen, Hannes Reinecke, Jens Axboe, linux-block
blkdev_issue_zeroout() will use this in !BLKDEV_ZERO_NOFALLBACK case.
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
block/blk-lib.c | 63 +++++++++++++++++++++++++++++++++------------------------
1 file changed, 37 insertions(+), 26 deletions(-)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index 62240f8832ca..9d2ab8bba52a 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -274,6 +274,40 @@ static unsigned int __blkdev_sectors_to_bio_pages(sector_t nr_sects)
return min(pages, (sector_t)BIO_MAX_PAGES);
}
+static int __blkdev_issue_zero_pages(struct block_device *bdev,
+ sector_t sector, sector_t nr_sects, gfp_t gfp_mask,
+ struct bio **biop)
+{
+ struct request_queue *q = bdev_get_queue(bdev);
+ struct bio *bio = *biop;
+ int bi_size = 0;
+ unsigned int sz;
+
+ if (!q)
+ return -ENXIO;
+
+ while (nr_sects != 0) {
+ bio = next_bio(bio, __blkdev_sectors_to_bio_pages(nr_sects),
+ gfp_mask);
+ bio->bi_iter.bi_sector = sector;
+ bio_set_dev(bio, bdev);
+ bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
+
+ while (nr_sects != 0) {
+ sz = min((sector_t) PAGE_SIZE, nr_sects << 9);
+ bi_size = bio_add_page(bio, ZERO_PAGE(0), sz, 0);
+ nr_sects -= bi_size >> 9;
+ sector += bi_size >> 9;
+ if (bi_size < sz)
+ break;
+ }
+ cond_resched();
+ }
+
+ *biop = bio;
+ return 0;
+}
+
/**
* __blkdev_issue_zeroout - generate number of zero filed write bios
* @bdev: blockdev to issue
@@ -304,9 +338,6 @@ int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
unsigned flags)
{
int ret;
- int bi_size = 0;
- struct bio *bio = *biop;
- unsigned int sz;
sector_t bs_mask;
bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
@@ -316,30 +347,10 @@ int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
ret = __blkdev_issue_write_zeroes(bdev, sector, nr_sects, gfp_mask,
biop, flags);
if (ret != -EOPNOTSUPP || (flags & BLKDEV_ZERO_NOFALLBACK))
- goto out;
-
- ret = 0;
- while (nr_sects != 0) {
- bio = next_bio(bio, __blkdev_sectors_to_bio_pages(nr_sects),
- gfp_mask);
- bio->bi_iter.bi_sector = sector;
- bio_set_dev(bio, bdev);
- bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
-
- while (nr_sects != 0) {
- sz = min((sector_t) PAGE_SIZE, nr_sects << 9);
- bi_size = bio_add_page(bio, ZERO_PAGE(0), sz, 0);
- nr_sects -= bi_size >> 9;
- sector += bi_size >> 9;
- if (bi_size < sz)
- break;
- }
- cond_resched();
- }
+ return ret;
- *biop = bio;
-out:
- return ret;
+ return __blkdev_issue_zero_pages(bdev, sector, nr_sects, gfp_mask,
+ biop);
}
EXPORT_SYMBOL(__blkdev_issue_zeroout);
--
2.4.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
2017-10-16 13:59 [PATCH v3 0/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout() Ilya Dryomov
2017-10-16 13:59 ` [PATCH v3 1/2] block: factor out __blkdev_issue_zero_pages() Ilya Dryomov
@ 2017-10-16 13:59 ` Ilya Dryomov
2017-10-18 6:35 ` Christoph Hellwig
2017-10-18 22:48 ` Martin K. Petersen
2017-10-25 6:27 ` [PATCH v3 0/2] " Ilya Dryomov
2 siblings, 2 replies; 9+ messages in thread
From: Ilya Dryomov @ 2017-10-16 13:59 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Martin K. Petersen, Hannes Reinecke, Jens Axboe, linux-block
sd_config_write_same() ignores ->max_ws_blocks == 0 and resets it to
permit trying WRITE SAME on older SCSI devices, unless ->no_write_same
is set. Because REQ_OP_WRITE_ZEROES is implemented in terms of WRITE
SAME, blkdev_issue_zeroout() may fail with -EREMOTEIO:
$ fallocate -zn -l 1k /dev/sdg
fallocate: fallocate failed: Remote I/O error
$ fallocate -zn -l 1k /dev/sdg # OK
$ fallocate -zn -l 1k /dev/sdg # OK
The following calls succeed because sd_done() sets ->no_write_same in
response to a sense that would become BLK_STS_TARGET/-EREMOTEIO, causing
__blkdev_issue_zeroout() to fall back to generating ZERO_PAGE bios.
This means blkdev_issue_zeroout() must cope with WRITE ZEROES failing
and fall back to manually zeroing, unless BLKDEV_ZERO_NOFALLBACK is
specified. For BLKDEV_ZERO_NOFALLBACK case, return -EOPNOTSUPP if
sd_done() has just set ->no_write_same thus indicating lack of offload
support.
Fixes: c20cfc27a473 ("block: stop using blkdev_issue_write_same for zeroing")
Cc: Christoph Hellwig <hch@lst.de>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
block/blk-lib.c | 45 +++++++++++++++++++++++++++++++++++----------
1 file changed, 35 insertions(+), 10 deletions(-)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index 9d2ab8bba52a..23411ec7f3cc 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -321,12 +321,6 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev,
* Zero-fill a block range, either using hardware offload or by explicitly
* writing zeroes to the device.
*
- * Note that this function may fail with -EOPNOTSUPP if the driver signals
- * zeroing offload support, but the device fails to process the command (for
- * some devices there is no non-destructive way to verify whether this
- * operation is actually supported). In this case the caller should call
- * retry the call to blkdev_issue_zeroout() and the fallback path will be used.
- *
* If a device is using logical block provisioning, the underlying space will
* not be released if %flags contains BLKDEV_ZERO_NOUNMAP.
*
@@ -370,18 +364,49 @@ EXPORT_SYMBOL(__blkdev_issue_zeroout);
int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask, unsigned flags)
{
- int ret;
- struct bio *bio = NULL;
+ int ret = 0;
+ sector_t bs_mask;
+ struct bio *bio;
struct blk_plug plug;
+ bool try_write_zeroes = !!bdev_write_zeroes_sectors(bdev);
+ bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
+ if ((sector | nr_sects) & bs_mask)
+ return -EINVAL;
+
+retry:
+ bio = NULL;
blk_start_plug(&plug);
- ret = __blkdev_issue_zeroout(bdev, sector, nr_sects, gfp_mask,
- &bio, flags);
+ if (try_write_zeroes) {
+ ret = __blkdev_issue_write_zeroes(bdev, sector, nr_sects,
+ gfp_mask, &bio, flags);
+ } else if (!(flags & BLKDEV_ZERO_NOFALLBACK)) {
+ ret = __blkdev_issue_zero_pages(bdev, sector, nr_sects,
+ gfp_mask, &bio);
+ } else {
+ /* No zeroing offload support */
+ ret = -EOPNOTSUPP;
+ }
if (ret == 0 && bio) {
ret = submit_bio_wait(bio);
bio_put(bio);
}
blk_finish_plug(&plug);
+ if (ret && try_write_zeroes) {
+ if (!(flags & BLKDEV_ZERO_NOFALLBACK)) {
+ try_write_zeroes = false;
+ goto retry;
+ }
+ if (!bdev_write_zeroes_sectors(bdev)) {
+ /*
+ * Zeroing offload support was indicated, but the
+ * device reported ILLEGAL REQUEST (for some devices
+ * there is no non-destructive way to verify whether
+ * WRITE ZEROES is actually supported).
+ */
+ ret = -EOPNOTSUPP;
+ }
+ }
return ret;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] block: factor out __blkdev_issue_zero_pages()
2017-10-16 13:59 ` [PATCH v3 1/2] block: factor out __blkdev_issue_zero_pages() Ilya Dryomov
@ 2017-10-18 6:35 ` Christoph Hellwig
2017-10-18 22:40 ` Martin K. Petersen
1 sibling, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-10-18 6:35 UTC (permalink / raw)
To: Ilya Dryomov
Cc: Christoph Hellwig, Martin K. Petersen, Hannes Reinecke,
Jens Axboe, linux-block
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
2017-10-16 13:59 ` [PATCH v3 2/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout() Ilya Dryomov
@ 2017-10-18 6:35 ` Christoph Hellwig
2017-10-18 22:48 ` Martin K. Petersen
1 sibling, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-10-18 6:35 UTC (permalink / raw)
To: Ilya Dryomov
Cc: Christoph Hellwig, Martin K. Petersen, Hannes Reinecke,
Jens Axboe, linux-block
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] block: factor out __blkdev_issue_zero_pages()
2017-10-16 13:59 ` [PATCH v3 1/2] block: factor out __blkdev_issue_zero_pages() Ilya Dryomov
2017-10-18 6:35 ` Christoph Hellwig
@ 2017-10-18 22:40 ` Martin K. Petersen
1 sibling, 0 replies; 9+ messages in thread
From: Martin K. Petersen @ 2017-10-18 22:40 UTC (permalink / raw)
To: Ilya Dryomov
Cc: Christoph Hellwig, Martin K. Petersen, Hannes Reinecke,
Jens Axboe, linux-block
Ilya,
> blkdev_issue_zeroout() will use this in !BLKDEV_ZERO_NOFALLBACK case.
Looks good to me.
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
2017-10-16 13:59 ` [PATCH v3 2/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout() Ilya Dryomov
2017-10-18 6:35 ` Christoph Hellwig
@ 2017-10-18 22:48 ` Martin K. Petersen
1 sibling, 0 replies; 9+ messages in thread
From: Martin K. Petersen @ 2017-10-18 22:48 UTC (permalink / raw)
To: Ilya Dryomov
Cc: Christoph Hellwig, Martin K. Petersen, Hannes Reinecke,
Jens Axboe, linux-block
Ilya,
> sd_config_write_same() ignores ->max_ws_blocks == 0 and resets it to
I'm not so keen on that description.
sd is the entity that owns max_ws_blocks so it certainly doesn't ignore
it. The default is for max_ws_blocks to be set because we have no
generic, non-destructive way of finding out whether a WRITE SAME command
is going to work or not. If a WRITE SAME subsequently fails, we'll
disable the feature by clearing max_write_same_sectors in the queue
limits.
> permit trying WRITE SAME on older SCSI devices, unless ->no_write_same
> is set. Because REQ_OP_WRITE_ZEROES is implemented in terms of WRITE
> SAME, blkdev_issue_zeroout() may fail with -EREMOTEIO:
Other than that patch looks good.
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
2017-10-16 13:59 [PATCH v3 0/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout() Ilya Dryomov
2017-10-16 13:59 ` [PATCH v3 1/2] block: factor out __blkdev_issue_zero_pages() Ilya Dryomov
2017-10-16 13:59 ` [PATCH v3 2/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout() Ilya Dryomov
@ 2017-10-25 6:27 ` Ilya Dryomov
2017-10-25 18:28 ` Jens Axboe
2 siblings, 1 reply; 9+ messages in thread
From: Ilya Dryomov @ 2017-10-25 6:27 UTC (permalink / raw)
To: Jens Axboe
Cc: Martin K. Petersen, Hannes Reinecke, linux-block, Christoph Hellwig
On Mon, Oct 16, 2017 at 3:59 PM, Ilya Dryomov <idryomov@gmail.com> wrote:
> Hi Christoph, Martin,
>
> blkdev_issue_zeroout() now checks for any error. This required a minor
> refactor, so I dropped the stable tag, Jens can add it back if needed.
>
> v2 -> v3:
> - another code flow change in blkdev_issue_zeroout() suggested by
> Christoph -- no functional changes
>
> v1 -> v2:
> - changed code flow in blkdev_issue_zeroout() according to Christoph's
> suggestion
> - this required adding additional checks to blkdev_issue_zeroout() and
> __blkdev_issue_zero_pages(), the latter is no longer void
>
> Previous version at
>
> https://marc.info/?l=linux-block&m=150712940922089&w=2
>
> Thanks,
>
> Ilya
>
>
> Ilya Dryomov (2):
> block: factor out __blkdev_issue_zero_pages()
> block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
>
> block/blk-lib.c | 108 +++++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 72 insertions(+), 36 deletions(-)
Jens, can you pick these up? Technically this is a user-visible
regression (Fixes tag in 2/2), but we are late in the cycle so I'm not
sure what is the right thing to do here.
Thanks,
Ilya
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
2017-10-25 6:27 ` [PATCH v3 0/2] " Ilya Dryomov
@ 2017-10-25 18:28 ` Jens Axboe
0 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2017-10-25 18:28 UTC (permalink / raw)
To: Ilya Dryomov
Cc: Martin K. Petersen, Hannes Reinecke, linux-block, Christoph Hellwig
On 10/24/2017 11:27 PM, Ilya Dryomov wrote:
> On Mon, Oct 16, 2017 at 3:59 PM, Ilya Dryomov <idryomov@gmail.com> wrote:
>> Hi Christoph, Martin,
>>
>> blkdev_issue_zeroout() now checks for any error. This required a minor
>> refactor, so I dropped the stable tag, Jens can add it back if needed.
>>
>> v2 -> v3:
>> - another code flow change in blkdev_issue_zeroout() suggested by
>> Christoph -- no functional changes
>>
>> v1 -> v2:
>> - changed code flow in blkdev_issue_zeroout() according to Christoph's
>> suggestion
>> - this required adding additional checks to blkdev_issue_zeroout() and
>> __blkdev_issue_zero_pages(), the latter is no longer void
>>
>> Previous version at
>>
>> https://marc.info/?l=linux-block&m=150712940922089&w=2
>>
>> Thanks,
>>
>> Ilya
>>
>>
>> Ilya Dryomov (2):
>> block: factor out __blkdev_issue_zero_pages()
>> block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
>>
>> block/blk-lib.c | 108 +++++++++++++++++++++++++++++++++++++-------------------
>> 1 file changed, 72 insertions(+), 36 deletions(-)
>
> Jens, can you pick these up? Technically this is a user-visible
> regression (Fixes tag in 2/2), but we are late in the cycle so I'm not
> sure what is the right thing to do here.
It wasn't introduced in this cycle, so there's little point in trying
to rush it for 4.14. I have applied it for 4.15, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-10-25 18:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-16 13:59 [PATCH v3 0/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout() Ilya Dryomov
2017-10-16 13:59 ` [PATCH v3 1/2] block: factor out __blkdev_issue_zero_pages() Ilya Dryomov
2017-10-18 6:35 ` Christoph Hellwig
2017-10-18 22:40 ` Martin K. Petersen
2017-10-16 13:59 ` [PATCH v3 2/2] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout() Ilya Dryomov
2017-10-18 6:35 ` Christoph Hellwig
2017-10-18 22:48 ` Martin K. Petersen
2017-10-25 6:27 ` [PATCH v3 0/2] " Ilya Dryomov
2017-10-25 18:28 ` Jens Axboe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.