On Fri, Oct 6, 2017 at 2:05 PM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> On Thu, Oct 05, 2017 at 09:32:33PM +0200, Ilya Dryomov wrote:
>> This is to avoid returning -EREMOTEIO in the following case: device
>> doesn't support WRITE SAME but scsi_disk::max_ws_blocks != 0, zeroout
>> is called with BLKDEV_ZERO_NOFALLBACK. Enter blkdev_issue_zeroout(),
>> bdev_write_zeroes_sectors() != 0, so we issue WRITE ZEROES. The
>> request fails with ILLEGAL REQUEST, sd_done() sets ->no_write_same and
>> updates queue_limits, ILLEGAL REQUEST is translated into -EREMOTEIO,
>> which is returned from submit_bio_wait(). Manual zeroing is not
>> allowed, so we must return an error, but it shouldn't be -EREMOTEIO if
>> queue_limits just got updated because of ILLEGAL REQUEST. Without this
>> conditional, we'd get
>
> Hmm. I think we'd better off to just do the before the retry loop:
>
> if (ret && try_write_zeroes) {
> if (!(flags & BLKDEV_ZERO_NOFALLBACK))
> try_write_zeroes = false;
> goto retry;
> }
> ret = -EOPNOTSUPP;
> }
This would unconditionally overwrite any WRITE ZEROS error. If we get
e.g. -EIO, and manual zeroing is not allowed, I don't think we want to
return -EOPNOTSUPP?
Returning -EOPNOTSUPP to mean "can't zero using either method" doesn't
make sense to me, because manual zeroing is always supported, just not
always allowed.
Thanks,
Ilya
