Re: [PATCH 3/8] iomap: treat a write through cache the same as FUA

[Jens Axboe <axboe@kernel.dk>]

On 7/21/23 9:55?AM, Darrick J. Wong wrote:
> On Fri, Jul 21, 2023 at 08:04:19AM -0600, Jens Axboe wrote:
>> On 7/21/23 12:15?AM, Christoph Hellwig wrote:
>>> On Thu, Jul 20, 2023 at 12:13:05PM -0600, Jens Axboe wrote:
>>>> Whether we have a write back cache and are using FUA or don't have
>>>> a write back cache at all is the same situation. Treat them the same.
>>>>
>>>> This makes the IOMAP_DIO_WRITE_FUA name a bit misleading, as we have
>>>> two cases that provide stable writes:
>>>>
>>>> 1) Volatile write cache with FUA writes
>>>> 2) Normal write without a volatile write cache
>>>>
>>>> Rename that flag to IOMAP_DIO_STABLE_WRITE to make that clearer, and
>>>> update some of the FUA comments as well.
>>>
>>> I would have preferred IOMAP_DIO_WRITE_THROUGH, STABLE_WRITES is a flag
>>> we use in file systems and the page cache for cases where the page
>>> can't be touched before writeback has completed, e.g.
>>> QUEUE_FLAG_STABLE_WRITES and SB_I_STABLE_WRITES.
>>
>> Good point, it does confuse terminology with stable pages for writes.
>> I'll change it to WRITE_THROUGH, that is more descriptive for this case.
> 
> +1 for the name change.
> 
> With IOMAP_DIO_WRITE_THROUGH,
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>

Thanks, I did make that change.

> Separately: At some point, the definition for IOMAP_DIO_DIRTY needs to
> grow a type annotation:
> 
> #define IOMAP_DIO_DIRTY		(1U << 31)
> 
> due (apparently) triggering UBSAN because "1" on its own is a signed
> constant.  If this series goes through my tree then I'll add a trivial
> patch fixing all of this ... unless you'd rather do it yourself as a
> patch 9?

Ah yes. I can add a patch for that and send out a v5. Will run the usual
testing on it with that patch added, then ship it out. Risk of conflict
with io_uring changes is pretty small, so would be fine to stage through
your tree.

-- 
Jens Axboe