Re: [PATCH 01/11] fuse: convert readahead to use folios

[Josef Bacik <josef@toxicpanda.com>]

On Tue, Aug 27, 2024 at 10:46:57PM +0100, Matthew Wilcox wrote:
> On Tue, Aug 27, 2024 at 04:45:14PM -0400, Josef Bacik wrote:
> >  
> > +/*
> > + * Wait for page writeback in the range to be completed.  This will work for
> > + * folio_size() > PAGE_SIZE, even tho we don't currently allow that.
> > + */
> > +static void fuse_wait_on_folio_writeback(struct inode *inode,
> > +					 struct folio *folio)
> > +{
> > +	for (pgoff_t index = folio_index(folio);
> > +	     index < folio_next_index(folio); index++)
> > +		fuse_wait_on_page_writeback(inode, index);
> > +}
> 
> Would it be better to write this as:
> 
> 	struct fuse_inode *fi = get_fuse_inode(inode);
> 	pgoff_t last = folio_next_index(folio) - 1;
> 
> 	wait_event(fi->page_waitq, !fuse_range_is_writeback(inode,
> 				folio->index, last));
> 
> > @@ -1015,13 +1036,14 @@ static void fuse_readahead(struct readahead_control *rac)
> >  		if (!ia)
> >  			return;
> >  		ap = &ia->ap;
> > -		nr_pages = __readahead_batch(rac, ap->pages, nr_pages);
> > -		for (i = 0; i < nr_pages; i++) {
> > -			fuse_wait_on_page_writeback(inode,
> > -						    readahead_index(rac) + i);
> > -			ap->descs[i].length = PAGE_SIZE;
> > +
> > +		while (nr_folios < nr_pages &&
> > +		       (folio = readahead_folio(rac)) != NULL) {
> > +			fuse_wait_on_folio_writeback(inode, folio);
> 
> Oh.  Even easier, we should hoist the whole thing to here.  Before
> this loop,
> 
> 		pgoff_t first = readahead_index(rac);
> 		pgoff_t last = first + readahead_count(rac) - 1;
> 		wait_event(fi->page_waitq, !fuse_range_is_writeback(inode,
> 				first, last);
> 
> (I'm not quite sure how we might have pending writeback still when we're
> doing readahead, but fuse is a funny creature and if somebody explains
> why to me, I'll probably forget again)
> 

Ah good suggestion, I like this better.  I didn't read carefully enough and
thought the waitqueue was on the writeback struct.  I'll rework it in the
morning and re-send once the tests run again.

> > +			ap->pages[i] = &folio->page;
> > +			ap->descs[i].length = folio_size(folio);
> > +			ap->num_pages++;
> 
> I do want to turn __readahead_batch into working on folios, but that
> involves working on fuse & squashfs at the same time ... I see you
> got rid of the readahead_page_batch() in btrfs recently; that'll help.

Do you want me to tackle that since I'm messing around in this area anyway?  My
only hesitation is we're limited to the 32 folios or whatever the pagevec count
is nowadays, and we may want to cycle through more.  But I've just finished
eating dinner and haven't actually looked at anything yet.  Thanks,

Josef