On Tue, Nov 12, 2024 at 10:16:10AM -0700, Jens Axboe wrote: > On 11/12/24 9:37 AM, Brian Foster wrote: > > On Mon, Nov 11, 2024 at 04:37:40PM -0700, Jens Axboe wrote: > >> Add iomap buffered write support for RWF_UNCACHED. If RWF_UNCACHED is > >> set for a write, mark the folios being written with drop_writeback. Then > > > > s/drop_writeback/uncached/ ? > > Ah indeed, guess that never got changed. Thanks, will fix that in the > commit message. > > > BTW, this might be getting into wonky "don't care that much" territory, > > but something else to be aware of is that certain writes can potentially > > change pagecache state as a side effect outside of the actual buffered > > write itself. > > > > For example, xfs calls iomap_zero_range() on write extension (i.e. pos > > > isize), which uses buffered writes and thus could populate a pagecache > > folio without setting it uncached, even if done on behalf of an uncached > > write. > > > > I've only made a first pass and could be missing some details, but IIUC > > I _think_ this means something like writing out a stream of small, > > sparse and file extending uncached writes could actually end up behaving > > more like sync I/O. Again, not saying that's something we really care > > about, just raising it in case it's worth considering or documenting.. > > No that's useful info, I'm not really surprised that there would still > be cases where UNCACHED goes unnoticed. In other words, I'd be surprised > if the current patches for eg xfs/ext4 cover all the cases where new > folios are created and should be marked as UNCACHED of IOCB_UNCACHED is > set in the iocb. > > I think those can be sorted out or documented as we move forward. > UNCACHED is really just a hint - the kernel should do its best to not > have permanent folios for this IO, but there are certainly cases where > it won't be honored if you're racing with regular buffered IO or mmap. > For the case above, sounds like we could cover that, however, and > probably should. > Ok. I suppose you could plumb the iocb state through the zero range call as well, but given the description/semantics I wouldn't blame you if you wanted to leave that for a followon improvement. Thanks for the explanation. Brian > -- > Jens Axboe >