Re: [PATCH] block: introduce BLKDEV_DISCARD_ZERO to fix zeroout

[Christoph Hellwig <hch@lst.de>]

On Tue, Jun 21, 2016 at 10:02:10PM -0400, Martin K. Petersen wrote:
> In 9082e87bfbf8 ("block: remove struct bio_batch") you changed the error
> return for blkdev_issue_write_same() to look like the one for discard:
> 
> -	if (bb.error)
> -		return bb.error;
> -	return ret;
> +	if (bio)
> +		ret = submit_bio_wait(REQ_WRITE | REQ_WRITE_SAME, bio);
> +	return ret != -EOPNOTSUPP ? ret : 0;
> 
> It's not a problem for SCSI since I'd be returning -EREMOTEIO. But if a
> stacking driver returns -EOPNOTSUPP in response to a WRITE SAME request
> we'll get data corruption.

Oh, I see what you mean, but I disagree with the analysis.  Unlike
discard outside the zeroout path, write same is a data integrity operation.
Just like in the zero out case turning an EOPNOTSUPP into 0 will get you
data corruption, as the caller will see a successful return for an
operation that did not actually write data to disk.  Same is true for
writing the actual zeroes in __blkdev_issue_zeroout.

Re stacking drivers and discard / write same:  why does
blk_set_stacking_limits set discard_zeroes_data to 1 and
max_write_same_sectors to UINT_MAX?  These seem like inherently dangerous
defaults.