Re: [PATCH v3 04/16] iomap: hide ioends from the generic writeback code

[Christoph Hellwig <hch@lst.de>]

On Wed, Jul 02, 2025 at 10:38:19AM -0700, Darrick J. Wong wrote:
> > -Filesystems that need to update internal bookkeeping (e.g. unwritten
> > -extent conversions) should provide a ``->submit_ioend`` function to
> > -set ``struct iomap_end::bio::bi_end_io`` to its own function.
> > -This function should call ``iomap_finish_ioends`` after finishing its
> > -own work (e.g. unwritten extent conversion).
> > -
> 
> I really wish you wouldn't delete the documentation that talks about
> what sort of things you might do in a ->writeback_submit function.
> That might be obvious to us who've been around for a long time, but I
> don't think that's so obvious to the junior programmers.

Because it's somewhere between wrong and totally arbitrary.  That whole
file is a real pain in the but because of that approach and I really
should have fought against adding it much harder.

> >  /*
> > - * Submit an ioend.
> 
> Please retain the summary.

The summary is split up into comments in the places where it makes
sense now.

> >  
> > -	if (!wpc->ioend || !iomap_can_add_to_ioend(wpc, pos, ioend_flags)) {
> > +	if (!ioend || !iomap_can_add_to_ioend(wpc, pos, ioend_flags)) {
> >  new_ioend:
> > -		error = iomap_submit_ioend(wpc, 0);
> > -		if (error)
> > -			return error;
> > -		wpc->ioend = iomap_alloc_ioend(wpc, pos, ioend_flags);
> > +		if (ioend) {
> > +			error = wpc->ops->writeback_submit(wpc, 0);
> 
> Should we call ioend_writeback_submit directly if
> !wpc->ops->writeback_submit, to avoid the indirect call hit for simpler
> filesystems?

No.  Compared to all the other indirect calls here it doesn't matter,
and having arbitrary defaults tends to cause problems down the road.