Christoph Hellwig
2020-Feb-20 15:47 UTC
[Ocfs2-devel] [PATCH v7 21/24] iomap: Restructure iomap_readpages_actor
On Wed, Feb 19, 2020 at 01:01:00PM -0800, Matthew Wilcox wrote:> From: "Matthew Wilcox (Oracle)" <willy at infradead.org> > > By putting the 'have we reached the end of the page' condition at the end > of the loop instead of the beginning, we can remove the 'submit the last > page' code from iomap_readpages(). Also check that iomap_readpage_actor() > didn't return 0, which would lead to an endless loop.I'm obviously biassed a I wrote the original code, but I find the new very much harder to understand (not that the previous one was easy, this is tricky code..).
Matthew Wilcox
2020-Feb-20 16:24 UTC
[Ocfs2-devel] [PATCH v7 21/24] iomap: Restructure iomap_readpages_actor
On Thu, Feb 20, 2020 at 07:47:41AM -0800, Christoph Hellwig wrote:> On Wed, Feb 19, 2020 at 01:01:00PM -0800, Matthew Wilcox wrote: > > From: "Matthew Wilcox (Oracle)" <willy at infradead.org> > > > > By putting the 'have we reached the end of the page' condition at the end > > of the loop instead of the beginning, we can remove the 'submit the last > > page' code from iomap_readpages(). Also check that iomap_readpage_actor() > > didn't return 0, which would lead to an endless loop. > > I'm obviously biassed a I wrote the original code, but I find the new > very much harder to understand (not that the previous one was easy, this > is tricky code..).Agreed, I found the original code hard to understand. I think this is easier because now cur_page doesn't leak outside this loop, so it has an obvious lifecycle. I'm kind of optimistic for Dave Howells' iov_iter addition of an ITER_MAPPING. That might simplify all of this code.