Re: [PATCH 2/8] iomap: simplify io_flags and io_type in struct iomap_ioend

[Christoph Hellwig <hch@lst.de>]

On Thu, Dec 12, 2024 at 09:55:04AM -0800, Darrick J. Wong wrote:
> > +#define IOMAP_IOEND_NOMERGE_FLAGS \
> > +	(IOMAP_IOEND_SHARED | IOMAP_IOEND_UNWRITTEN)
> 
> Hmm.  At first I wondered "Why wouldn't BOUNDARY be in here too?  It
> also prevents merging of ioends."  Then I remembered that BOUNDARY is an
> explicit nomerge flag, whereas what NOMERGE_FLAGS provides is that we
> always split ioends whenever the ioend work changes.
> 
> How about a comment?
> 
> /* split ioends when the type of completion work changes */
> #define IOMAP_IOEND_NOMERGE_FLAGS \
> 	(IOMAP_IOEND_SHARED | IOMAP_IOEND_UNWRITTEN)
> 
> Otherwise this looks fine to me.

The interesting thing about BOUNDARY is not just that it's explicit, but
also that it's one-way.  We can merge a non-BOUNDARY flag into the end
of a BOUNDARY one, just not a BOUNDARY one into the end of a non-BOUNDARY
one.

> 
> --D
> 
> > +
> >  /*
> >   * Structure for writeback I/O completions.
> >   */
> >  struct iomap_ioend {
> >  	struct list_head	io_list;	/* next ioend in chain */
> > -	u16			io_type;
> > -	u16			io_flags;	/* IOMAP_F_* */
> > +	u16			io_flags;	/* IOMAP_IOEND_* */
> >  	struct inode		*io_inode;	/* file being written to */
> >  	size_t			io_size;	/* size of the extent */
> >  	loff_t			io_offset;	/* offset in the file */
> > -- 
> > 2.45.2
> > 
> > 
---end quoted text---