On Fri, Sep 27, 2024 at 03:22:25PM -0700, Joanne Koong wrote:
> On Fri, Sep 27, 2024 at 1:45 PM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
> >
> > Currently we're using the __readahead_batch() helper which populates our
> > fuse_args_pages->pages array with pages. Convert this to use the newer
> > folio based pattern which is to call readahead_folio() to get the next
> > folio in the read ahead batch. I've updated the code to use things like
> > folio_size() and to take into account larger folio sizes, but this is
> > purely to make that eventual work easier to do, we currently will not
> > get large folios so this is more future proofing than actual support.
> >
> > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> > ---
> > fs/fuse/file.c | 43 ++++++++++++++++++++++++++++---------------
> > 1 file changed, 28 insertions(+), 15 deletions(-)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index f33fbce86ae0..132528cde745 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -938,7 +938,6 @@ static void fuse_readpages_end(struct fuse_mount *fm, struct fuse_args *args,
> > struct folio *folio = page_folio(ap->pages[i]);
> >
> > folio_end_read(folio, !err);
> > - folio_put(folio);
> > }
> > if (ia->ff)
> > fuse_file_put(ia->ff, false);
> > @@ -985,18 +984,36 @@ static void fuse_send_readpages(struct fuse_io_args *ia, struct file *file)
> > static void fuse_readahead(struct readahead_control *rac)
> > {
> > struct inode *inode = rac->mapping->host;
> > + struct fuse_inode *fi = get_fuse_inode(inode);
> > struct fuse_conn *fc = get_fuse_conn(inode);
> > - unsigned int i, max_pages, nr_pages = 0;
> > + unsigned int max_pages, nr_pages;
> > + pgoff_t first = readahead_index(rac);
> > + pgoff_t last = first + readahead_count(rac) - 1;
> >
> > if (fuse_is_bad(inode))
> > return;
> >
> > + wait_event(fi->page_waitq, !fuse_range_is_writeback(inode, first, last));
>
> Should this line be moved to after we check the readahead count? eg
>
> nr_pages = readahead_count(rac);
> if (!nr_pages)
> return;
> wait_event(fi->page_waitq, !fuse_range_is_writeback(inode, first, last));
>
> Otherwise I think in that case you mentioned where read_pages() calls
> into readahead_folio() after it's consumed the last folio, we end up
> calling this wait_event
The first bit of read_pages covers this for us
static void read_pages(struct readahead_control *rac)
{
const struct address_space_operations *aops = rac->mapping->a_ops;
struct folio *folio;
struct blk_plug plug;
if (!readahead_count(rac))
return;
We don't get ->readahead called unless there's pages to read. Thanks,
Josef
