Re: [PATCH block v2 2/3] block: Add support for REQ_NOZERO flag

Christoph Hellwig
2020-01-31T06:23:54Z

On Tue, Jan 21, 2020 at 01:14:05AM -0500, Martin K. Petersen wrote:
> I find there is some dissonance between using BLKDEV_ZERO_ALLOCATE to
> describe this operation in one case and REQ_NOZERO in the other.
>
> I understand why not zeroing is important in your case. However, I think
> the allocation aspect is semantically more important. Also, in the case
> of SCSI, the allocated blocks will typically appear zeroed. So from that
> perspective REQ_NOZERO doesn't really make sense. I would really prefer
> to use REQ_ALLOCATE to describe this operation. I agree that "do not
> write every block" is important too. I just don't have a good suggestion
> for how to express that as an additional qualifier to REQ_ALLOCATE_?.

Agreed. Nevermind the problem of a REQ_OP_WRITE_ZEROES operations with
a NOZERO flag causing a massive confusion to the reader.

> Also, adding to the confusion: In the context of SCSI, ANCHOR requires
> UNMAP. So my head hurts a bit when I read REQ_NOZERO|REQ_NOUNMAP and
> have to translate that into ANCHOR|UNMAP.
>
> Longer term, I think we should consider introducing REQ_OP_SINGLE_RANGE
> or something like that as an umbrella operation that can be used to
> describe zeroing, allocating, and other things that operate on a single
> LBA range with no payload. Thus removing both the writiness and the
> zeroness from the existing REQ_OP_WRITE_ZEROES conduit.

What is the benefit of a multipler there? Given all this flags
confusion I'm almost tempted to just split up REQ_OP_WRITE_ZEROES into
REQ_OP_ALLOCATE ("cheap") and REQ_OP_WRITE_ZEROES ("potentially
expensive") and just let the caller handle the difference. Everytime
we try to encode semantic differences into flags we're eventually
running into trouble. Sais the person that added REQ_UNMAP..