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@xxxxxxxxxx> 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
