| Message ID | ffa6fe7ca63c4b2647447ddc9e5c1a67fe0fbb2d.1727469663.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fuse: folio conversions | expand |
On Fri, Sep 27, 2024 at 1:45 PM Josef Bacik <josef@toxicpanda.com> 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@toxicpanda.com> > --- > 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 > + > max_pages = min_t(unsigned int, fc->max_pages, > fc->max_read / PAGE_SIZE); > > - for (;;) { > + /* > + * This is only accurate the first time through, since readahead_folio() > + * doesn't update readahead_count() from the previous folio until the > + * next call. Grab nr_pages here so we know how many pages we're going > + * to have to process. This means that we will exit here with > + * readahead_count() == folio_nr_pages(last_folio), but we will have > + * consumed all of the folios, and read_pages() will call > + * readahead_folio() again which will clean up the rac. > + */ > + nr_pages = readahead_count(rac); > + > + while (nr_pages) { > struct fuse_io_args *ia; > struct fuse_args_pages *ap; > + struct folio *folio; > + unsigned cur_pages = min(max_pages, nr_pages); > > if (fc->num_background >= fc->congestion_threshold && > rac->ra->async_size >= readahead_count(rac)) > @@ -1006,23 +1023,19 @@ static void fuse_readahead(struct readahead_control *rac) > */ > break; > > - nr_pages = readahead_count(rac) - nr_pages; > - if (nr_pages > max_pages) > - nr_pages = max_pages; > - if (nr_pages == 0) > - break; > - ia = fuse_io_alloc(NULL, nr_pages); > + ia = fuse_io_alloc(NULL, cur_pages); > 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 (ap->num_pages < cur_pages && > + (folio = readahead_folio(rac)) != NULL) { > + ap->pages[ap->num_pages] = &folio->page; > + ap->descs[ap->num_pages].length = folio_size(folio); > + ap->num_pages++; > } > - ap->num_pages = nr_pages; > fuse_send_readpages(ia, rac->file); > + nr_pages -= cur_pages; > } > } > > -- > 2.43.0 > >
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@toxicpanda.com> 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@toxicpanda.com> > > --- > > 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
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)); + max_pages = min_t(unsigned int, fc->max_pages, fc->max_read / PAGE_SIZE); - for (;;) { + /* + * This is only accurate the first time through, since readahead_folio() + * doesn't update readahead_count() from the previous folio until the + * next call. Grab nr_pages here so we know how many pages we're going + * to have to process. This means that we will exit here with + * readahead_count() == folio_nr_pages(last_folio), but we will have + * consumed all of the folios, and read_pages() will call + * readahead_folio() again which will clean up the rac. + */ + nr_pages = readahead_count(rac); + + while (nr_pages) { struct fuse_io_args *ia; struct fuse_args_pages *ap; + struct folio *folio; + unsigned cur_pages = min(max_pages, nr_pages); if (fc->num_background >= fc->congestion_threshold && rac->ra->async_size >= readahead_count(rac)) @@ -1006,23 +1023,19 @@ static void fuse_readahead(struct readahead_control *rac) */ break; - nr_pages = readahead_count(rac) - nr_pages; - if (nr_pages > max_pages) - nr_pages = max_pages; - if (nr_pages == 0) - break; - ia = fuse_io_alloc(NULL, nr_pages); + ia = fuse_io_alloc(NULL, cur_pages); 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 (ap->num_pages < cur_pages && + (folio = readahead_folio(rac)) != NULL) { + ap->pages[ap->num_pages] = &folio->page; + ap->descs[ap->num_pages].length = folio_size(folio); + ap->num_pages++; } - ap->num_pages = nr_pages; fuse_send_readpages(ia, rac->file); + nr_pages -= cur_pages; } }
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@toxicpanda.com> --- fs/fuse/file.c | 43 ++++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-)