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

Darrick J. Wong
2023-07-21T15:55:36Z

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>

--D

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?

--D

> --
> Jens Axboe
>