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

From: Jens Axboe <axboe@kernel.dk>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>,
	io-uring@vger.kernel.org, linux-xfs@vger.kernel.org,
	andres@anarazel.de, david@fromorbit.com
Subject: Re: [PATCH 3/8] iomap: treat a write through cache the same as FUA
Date: Fri, 21 Jul 2023 10:03:06 -0600	[thread overview]
Message-ID: <e3c86a40-ad61-4c56-fd20-1afac7c90505@kernel.dk> (raw)
In-Reply-To: <20230721155506.GQ11352@frogsfrogsfrogs>

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