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.
