Re: [PATCH] Documentation: document the design of iomap and how to port

Jan Kara
2024-06-10T22:25:19Z

On Mon 10-06-24 14:59:28, Darrick J. Wong wrote:
> > > These struct kiocb flags are significant for buffered I/O with
> > > iomap:
> > >
> > > * IOCB_NOWAIT: Only proceed with the I/O if mapping data are
> > > already in memory, we do not have to initiate other I/O, and
> > > we acquire all filesystem locks without blocking. Neither
> > > this flag nor its definition RWF_NOWAIT actually define what
> > > this flag means, so this is the best the author could come
> > > up with.
> >
> > RWF_NOWAIT is a performance feature, not a correctness one, hence the
> > meaning is somewhat vague. It is meant to mean "do the IO only if it
> > doesn't involve waiting for other IO or other time expensive operations".
> > Generally we translate it to "don't wait for i_rwsem, page locks, don't do
> > block allocation, etc." OTOH we don't bother to specialcase internal
> > filesystem locks (such as EXT4_I(inode)->i_data_sem) and we get away with
> > it because blocking on it under constraints we generally perform RWF_NOWAIT
> > IO is exceedingly rare.
>
> I hate this flag's undocumented nature. It now makes *documenting*
> things around it hard. How about:
>
> "IOCB_NOWAIT: Neither this flag nor its associated definition RWF_NOWAIT
> actually specify what this flag means. Community members seem to think
> that it means only proceed with the I/O if it doesn't involve waiting
> for expensive operations. XFS and ext4 appear to reject the IO unless
> the mapping data are already in memory, the filesystem does not have to
> initiate other I/O, and the kernel can acquire all filesystem locks
> without blocking."

I guess this is good enough :)

> > > Direct Writes
> > >
> > > A direct I/O write initiates a write I/O to the storage device to
> > > the caller's buffer. Dirty parts of the pagecache are flushed to
> > > storage before initiating the write io. The pagecache is
> > > invalidated both before and after the write io. The flags value
> > > for ->iomap_begin will be IOMAP_DIRECT | IOMAP_WRITE with any
> > > combination of the following enhancements:
> > >
> > > * IOMAP_NOWAIT: Write if mapping data are already in memory.
> > > Does not initiate other I/O or block on filesystem locks.
> > >
> > > * IOMAP_OVERWRITE_ONLY: Allocating blocks and zeroing partial
> > > blocks is not allowed. The entire file range must to map to
> > ^^ extra "to"
> >
> > > a single written or unwritten extent. The file I/O range
> > > must be aligned to the filesystem block size.
> >
> > This seems to be XFS specific thing? At least I don't see anything in
> > generic iomap code depending on this?
>
> Hmm. XFS bails out if the mapping is unwritten and the directio write
> range isn't aligned to the fsblock size. I think the reason for that is
> because we'd have to zero the unaligned regions outside of the write
> range, and xfs can't do that without synchronizing. (Or we didn't think
> that was common enough to bother with the code complexity.)
>
> "The file I/O range must be aligned to the filesystem block size
> if the filesystem supports unwritten mappings but cannot zero unaligned
> regions without exposing stale contents."?

Sounds good.

Honza

--
Jan Kara <jack@suse.com>
SUSE Labs, CR