Re: [PATCH 2/3] iomap: decouple simple direct I/O reads from iomap_dio_rw
"Darrick J. Wong" <[email protected]>
| Newsgroups | org.kernel.vger.linux-ext4,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-xfs |
|---|---|
| Message-ID | <20260723151021.GC7380@frogsfrogsfrogs> |
On Thu, Jul 23, 2026 at 06:53:47AM +0200, Christoph Hellwig wrote: > On Wed, Jul 22, 2026 at 12:03:49PM -0700, Joanne Koong wrote: > > > +ssize_t __iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter, > > > + struct iomap_iter *iomi); > > > > Might be nice to have _read_ in this name as well, eg __iomap_dio_read_simple > > Yeah. I wanted the read out of the guts of the implementation, as > most of the code is not strictly limited to reads. But with the public > API needing it, we might as well be consistent in the function naming. > > > > + if (!ret) > > > + ret = __iomap_dio_simple(iocb, iter, &iomi); > > > + if (ret <= 0 && ret != -EIOCBQUEUED) > > > + inode_dio_end(iomi.inode); > > > > afaict, the inode_dio_end() error handling has to be done by > > __iomap_dio_simple() to properly account for the synchronous case. I > > ran into this earlier when doing the iomap_next_fn inlining. For the > > synchronous case, __iomap_dio_simple() calls > > iomap_dio_simple_complete() which calls inode_dio_end() even on > > errors. > > Yes. Sashiko also reported that yesterday and I replied with a patch, > but I now noticed the report and thus my reply only went to me, > a sashiko-review list and Ted, but not everone else or the lists. > > WTF? Sashiko doesn't email the lists by default, and I don't think we want to turn that on for the fsdevel firehose. I sorta wish it did, but we all seem still to be struggling with what to do with valid complaints about existing problems from sashiko that neither were introduced by the patchset nor critically impact the new code. In the meantime I also don't like the "private review involving someone else's dashboard" aspects either. No cake, no eating. :( --D