On Thu, Jan 30, 2020 at 01:35:33PM -0800, Matthew Wilcox wrote: > On Wed, Jan 29, 2020 at 12:08:29PM +1100, Dave Chinner wrote: > > On Fri, Jan 24, 2020 at 05:35:52PM -0800, Matthew Wilcox wrote: > > > + while (nr_pages--) { > > > + struct page *page = readahead_page(mapping, start++); > > > + int err = fuse_readpages_fill(&data, page); > > > + > > > + if (!err) > > > + continue; > > > + nr_pages++; > > > + goto out; > > > } > > > > That's some pretty convoluted logic. Perhaps: > > > > for (; nr_pages > 0 ; nr_pages--) { > > struct page *page = readahead_page(mapping, start++); > > > > if (fuse_readpages_fill(&data, page)) > > goto out; > > } > > I have a bit of an aversion to that style of for loop ... I like this > instead: > > while (nr_pages) { > struct page *page = readahead_page(mapping, start++); > > if (fuse_readpages_fill(&data, page) != 0) > goto out; > nr_pages--; > } yup, that's also fine. -Dave. -- Dave Chinner david@xxxxxxxxxxxxx