Re: [PATCH v8 10/10] nvme: Atomic write support

Christoph Hellwig
2024-06-18T06:49:13Z

On Mon, Jun 17, 2024 at 07:04:23PM +0100, John Garry wrote:
>> Nit: I'd cache blk_rq_bytes(req), since that is repeating and this
>> function is called for each atomic IO.
>
> blk_rq_bytes() is just a wrapper for rq->__data_len. I suppose that we
> could cache that value to stop re-reading that memory, but I would
> hope/expect that memory to be in the CPU cache anyway.

Yes, that feels a bit pointless.

> Only NVMe supports an LBA space boundary, so that part is specific to NVMe.
>
> Regardless, the block layer already should ensure that the atomic write
> length and boundary is respected. nvme_valid_atomic_write() is just an
> insurance policy against the block layer or some other component not doing
> its job.
>
> For SCSI, the device would error - for example - if the atomic write length
> was larger than the device supported. NVMe silently just does not execute
> the write atomically in that scenario, which we must avoid.

It might be worth to expand the comment to include this information to
help future readers.