Received-SPF: pass (google.com: domain of linux-ext4+bounces-2842-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Tue, 11 Jun 2024 00:25:12 +0200
From: Jan Kara <jack@suse.cz>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: Jan Kara <jack@suse.cz>, Christoph Hellwig <hch@infradead.org>,
	"Ritesh Harjani (IBM)" <ritesh.list@gmail.com>,
	linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, Dave Chinner <david@fromorbit.com>,
	Matthew Wilcox <willy@infradead.org>,
	Christian Brauner <brauner@kernel.org>,
	Ojaswin Mujoo <ojaswin@linux.ibm.com>,
	Luis Chamberlain <mcgrof@kernel.org>
Subject: Re: [PATCH] Documentation: document the design of iomap and how to
 port
Message-ID: <20240610222512.onus2iyje7fq3ic3@quack3>
References: <20240608001707.GD52973@frogsfrogsfrogs>
 <ZmVNblggFRgR8bnJ@infradead.org>
 <20240609155506.GT52987@frogsfrogsfrogs>
 <20240610141808.vdsflgcbjmgc37dt@quack3>
 <20240610215928.GV52987@frogsfrogsfrogs>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240610215928.GV52987@frogsfrogsfrogs>

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