Hi Xin, On 5/7/22 12:48 PM, Xin Yin wrote: > Use asyncronous io to read data from fscache may greatly improve IO > bandwidth for sequential buffer read scenario. > > Change erofs_fscache_read_folios to erofs_fscache_read_folios_async, > and read data from fscache asyncronously. Make .readpage()/.readahead() > to use this new helper. > > Signed-off-by: Xin Yin <yinxin.x@xxxxxxxxxxxxx> > --- > changes from RFC: > 1.rebase to fscache,erofs: fscache-based on-demand read semantics v10. > 2.fix issues pointed out by Jeffle. > 3.simplify parameters, add debug messages for erofs_fscache_read_folios_async. > 4.also change .readpage() to use new helper to avoid code duplication. > > I verified this patch introduces no regressions with tests in > https://github.com/lostjeffle/demand-read-cachefilesd. > --- > fs/erofs/fscache.c | 267 +++++++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 227 insertions(+), 40 deletions(-) > > diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c > index a402d8f0a063..2606bf4145f8 100644 > --- a/fs/erofs/fscache.c > +++ b/fs/erofs/fscache.c > @@ -5,57 +5,231 @@ > #include <linux/fscache.h> > #include "internal.h" > > +static void erofs_fscache_put_subrequest(struct netfs_io_subrequest *subreq); > + > +static struct netfs_io_request *erofs_fscache_alloc_request(struct address_space *mapping, > + loff_t start, size_t len) > +{ > + struct netfs_io_request *rreq; > + > + rreq = kzalloc(sizeof(struct netfs_io_request), GFP_KERNEL); > + if (!rreq) > + return ERR_PTR(-ENOMEM); > + > + rreq->start = start; > + rreq->len = len; > + rreq->mapping = mapping; > + INIT_LIST_HEAD(&rreq->subrequests); > + refcount_set(&rreq->ref, 1); > + > + return rreq; > +} > + > +static void erofs_fscache_clear_subrequests(struct netfs_io_request *rreq) > +{ > + struct netfs_io_subrequest *subreq; > + > + while (!list_empty(&rreq->subrequests)) { > + subreq = list_first_entry(&rreq->subrequests, > + struct netfs_io_subrequest, rreq_link); > + list_del(&subreq->rreq_link); > + erofs_fscache_put_subrequest(subreq); > + } > +} > + > +static void erofs_fscache_free_request(struct netfs_io_request *rreq) > +{ > + if (rreq->cache_resources.ops) > + rreq->cache_resources.ops->end_operation(&rreq->cache_resources); > + kfree(rreq); > +} > + > +static void erofs_fscache_put_request(struct netfs_io_request *rreq) > +{ > + if (refcount_dec_and_test(&rreq->ref)) > + erofs_fscache_free_request(rreq); > +} > + > + > +static struct netfs_io_subrequest * > + erofs_fscache_alloc_subrequest(struct netfs_io_request *rreq) > +{ > + struct netfs_io_subrequest *subreq; > + > + subreq = kzalloc(sizeof(struct netfs_io_subrequest), GFP_KERNEL); > + if (subreq) { > + INIT_LIST_HEAD(&subreq->rreq_link); > + refcount_set(&subreq->ref, 2); > + subreq->rreq = rreq; > + refcount_inc(&rreq->ref); > + } > + > + return subreq; > +} > + > +static void erofs_fscache_free_subrequest(struct netfs_io_subrequest *subreq) > +{ > + struct netfs_io_request *rreq = subreq->rreq; > + > + kfree(subreq); > + erofs_fscache_put_request(rreq); > +} Yeah, as suggested by Gao Xaing, we'd better fold this function in. Besides, we could use "erofs_fscache_put_request(subreq->rreq)" instead to avoid the definition of the local variable @rreq, so that the code arrangement could be more compact :) > + > +static void erofs_fscache_put_subrequest(struct netfs_io_subrequest *subreq) > +{ > + if (refcount_dec_and_test(&subreq->ref)) > + erofs_fscache_free_subrequest(subreq); > +} > + > + > +static void erofs_fscache_rreq_unlock_folios(struct netfs_io_request *rreq) > +{ > + struct netfs_io_subrequest *subreq; > + struct folio *folio; > + unsigned int iopos; > + pgoff_t start_page = rreq->start / PAGE_SIZE; > + pgoff_t last_page = ((rreq->start + rreq->len) / PAGE_SIZE) - 1; > + bool subreq_failed = false; > + > + XA_STATE(xas, &rreq->mapping->i_pages, start_page); > + > + subreq = list_first_entry(&rreq->subrequests, > + struct netfs_io_subrequest, rreq_link); > + iopos = 0; > + subreq_failed = (subreq->error < 0); > + > + rcu_read_lock(); > + xas_for_each(&xas, folio, last_page) { > + unsigned int pgpos = (folio_index(folio) - start_page) * PAGE_SIZE; > + unsigned int pgend = pgpos + folio_size(folio); > + bool pg_failed = false; > + > + for (;;) { > + if (!subreq) { > + pg_failed = true; > + break; > + } > + > + pg_failed |= subreq_failed; > + if (pgend < iopos + subreq->len) > + break; > + > + iopos += subreq->len; > + if (!list_is_last(&subreq->rreq_link, &rreq->subrequests)) { > + subreq = list_next_entry(subreq, rreq_link); > + subreq_failed = (subreq->error < 0); > + } else { > + subreq = NULL; > + subreq_failed = false; > + } > + if (pgend == iopos) > + break; > + } > + > + if (!pg_failed) > + folio_mark_uptodate(folio); > + > + folio_unlock(folio); > + } > + rcu_read_unlock(); > +} > + > + > +static void erofs_fscache_rreq_complete(struct netfs_io_request *rreq) > +{ > + erofs_fscache_rreq_unlock_folios(rreq); > + erofs_fscache_clear_subrequests(rreq); > + erofs_fscache_put_request(rreq); > +} > + > +static void erofc_fscache_subreq_complete(void *priv, ssize_t transferred_or_error, > + bool was_async) > +{ > + struct netfs_io_subrequest *subreq = priv; > + struct netfs_io_request *rreq = subreq->rreq; > + > + if (IS_ERR_VALUE(transferred_or_error)) > + subreq->error = transferred_or_error; > + > + if (atomic_dec_and_test(&rreq->nr_outstanding)) > + erofs_fscache_rreq_complete(rreq); > + > + erofs_fscache_put_subrequest(subreq); > +} > + > /* > * Read data from fscache and fill the read data into page cache described by > - * @start/len, which shall be both aligned with PAGE_SIZE. @pstart describes > + * @rreq, which shall be both aligned with PAGE_SIZE. @pstart describes > * the start physical address in the cache file. > */ > -static int erofs_fscache_read_folios(struct fscache_cookie *cookie, > - struct address_space *mapping, > - loff_t start, size_t len, > +static int erofs_fscache_read_folios_async(struct fscache_cookie *cookie, > + struct netfs_io_request *rreq, > loff_t pstart) > { > enum netfs_io_source source; > - struct netfs_io_request rreq = {}; > - struct netfs_io_subrequest subreq = { .rreq = &rreq, }; > - struct netfs_cache_resources *cres = &rreq.cache_resources; > - struct super_block *sb = mapping->host->i_sb; > + struct super_block *sb = rreq->mapping->host->i_sb; > + struct netfs_io_subrequest *subreq; > + struct netfs_cache_resources *cres; ^ How about assigning "cres = &rreq->cache_resources" directly here? > struct iov_iter iter; > + loff_t start = rreq->start; > + size_t len = rreq->len; > size_t done = 0; > int ret; > > + atomic_set(&rreq->nr_outstanding, 1); > + > + cres = &rreq->cache_resources; ^ As described, could be folded. > ret = fscache_begin_read_operation(cres, cookie); > if (ret) > - return ret; > + goto out; > > while (done < len) { > - subreq.start = pstart + done; > - subreq.len = len - done; > - subreq.flags = 1 << NETFS_SREQ_ONDEMAND; > + subreq = erofs_fscache_alloc_subrequest(rreq); > + if (!subreq) { > + ret = -ENOMEM; > + goto out; > + } > + > + subreq->start = pstart + done; > + subreq->len = len - done; > + subreq->flags = 1 << NETFS_SREQ_ONDEMAND; > > - source = cres->ops->prepare_read(&subreq, LLONG_MAX); > - if (WARN_ON(subreq.len == 0)) > + list_add_tail(&subreq->rreq_link, &rreq->subrequests); > + > + source = cres->ops->prepare_read(subreq, LLONG_MAX); > + if (WARN_ON(subreq->len == 0)) > source = NETFS_INVALID_READ; > if (source != NETFS_READ_FROM_CACHE) { > erofs_err(sb, "failed to fscache prepare_read (source %d)", > source); > ret = -EIO; > + subreq->error = ret; > + erofs_fscache_put_subrequest(subreq); > goto out; > } > > - iov_iter_xarray(&iter, READ, &mapping->i_pages, > - start + done, subreq.len); > - ret = fscache_read(cres, subreq.start, &iter, > - NETFS_READ_HOLE_FAIL, NULL, NULL); > + atomic_inc(&rreq->nr_outstanding); > + > + iov_iter_xarray(&iter, READ, &rreq->mapping->i_pages, > + start + done, subreq->len); > + > + ret = fscache_read(cres, subreq->start, &iter, > + NETFS_READ_HOLE_FAIL, erofc_fscache_subreq_complete, subreq); > + > + if (ret == -EIOCBQUEUED) > + ret = 0; > + > if (ret) { > erofs_err(sb, "failed to fscache_read (ret %d)", ret); > goto out; > } > > - done += subreq.len; > + done += subreq->len; > } > out: > - fscache_end_operation(cres); > + if (atomic_dec_and_test(&rreq->nr_outstanding)) > + erofs_fscache_rreq_complete(rreq); > + > return ret; > } > > @@ -64,6 +238,7 @@ static int erofs_fscache_meta_readpage(struct file *data, struct page *page) > int ret; > struct folio *folio = page_folio(page); > struct super_block *sb = folio_mapping(folio)->host->i_sb; > + struct netfs_io_request *rreq; > struct erofs_map_dev mdev = { > .m_deviceid = 0, > .m_pa = folio_pos(folio), > @@ -73,11 +248,13 @@ static int erofs_fscache_meta_readpage(struct file *data, struct page *page) > if (ret) > goto out; > > - ret = erofs_fscache_read_folios(mdev.m_fscache->cookie, > - folio_mapping(folio), folio_pos(folio), > - folio_size(folio), mdev.m_pa); > - if (!ret) > - folio_mark_uptodate(folio); > + rreq = erofs_fscache_alloc_request(folio_mapping(folio), > + folio_pos(folio), folio_size(folio)); > + if (IS_ERR(rreq)) > + goto out; > + > + return erofs_fscache_read_folios_async(mdev.m_fscache->cookie, > + rreq, mdev.m_pa); > out: > folio_unlock(folio); > return ret; > @@ -117,6 +294,7 @@ static int erofs_fscache_readpage(struct file *file, struct page *page) > struct super_block *sb = inode->i_sb; > struct erofs_map_blocks map; > struct erofs_map_dev mdev; > + struct netfs_io_request *rreq; > erofs_off_t pos; > loff_t pstart; > int ret; > @@ -149,10 +327,15 @@ static int erofs_fscache_readpage(struct file *file, struct page *page) > if (ret) > goto out_unlock; > > + > + rreq = erofs_fscache_alloc_request(folio_mapping(folio), > + folio_pos(folio), folio_size(folio)); > + if (IS_ERR(rreq)) > + goto out_unlock; > + > pstart = mdev.m_pa + (pos - map.m_la); > - ret = erofs_fscache_read_folios(mdev.m_fscache->cookie, > - folio_mapping(folio), folio_pos(folio), > - folio_size(folio), pstart); > + return erofs_fscache_read_folios_async(mdev.m_fscache->cookie, > + rreq, pstart); > > out_uptodate: > if (!ret) > @@ -162,15 +345,16 @@ static int erofs_fscache_readpage(struct file *file, struct page *page) > return ret; > } > > -static void erofs_fscache_unlock_folios(struct readahead_control *rac, > - size_t len) > +static void erofs_fscache_readahead_folios(struct readahead_control *rac, > + size_t len, bool unlock) > { > while (len) { > struct folio *folio = readahead_folio(rac); > - > len -= folio_size(folio); > - folio_mark_uptodate(folio); > - folio_unlock(folio); > + if (unlock) { > + folio_mark_uptodate(folio); > + folio_unlock(folio); > + } > } > } How about renaming this function into "erofs_fscache_advance_folios"? Since people may misunderstand that erofs_fscache_readahead_folios() is the main body of .readahead(). > > @@ -192,6 +376,7 @@ static void erofs_fscache_readahead(struct readahead_control *rac) > do { > struct erofs_map_blocks map; > struct erofs_map_dev mdev; > + struct netfs_io_request *rreq; > > pos = start + done; > map.m_la = pos; > @@ -211,7 +396,7 @@ static void erofs_fscache_readahead(struct readahead_control *rac) > offset, count); > iov_iter_zero(count, &iter); > > - erofs_fscache_unlock_folios(rac, count); > + erofs_fscache_readahead_folios(rac, count, true); > ret = count; > continue; > } > @@ -237,15 +422,17 @@ static void erofs_fscache_readahead(struct readahead_control *rac) > if (ret) > return; > > - ret = erofs_fscache_read_folios(mdev.m_fscache->cookie, > - rac->mapping, offset, count, > - mdev.m_pa + (pos - map.m_la)); > + rreq = erofs_fscache_alloc_request(rac->mapping, offset, count); > + if (IS_ERR(rreq)) > + return; > /* > - * For the error cases, the folios will be unlocked when > - * .readahead() returns. > + * Drop the ref of folios here. Unlock them in > + * rreq_unlock_folios() when rreq complete. > */ > + erofs_fscache_readahead_folios(rac, count, false); > + ret = erofs_fscache_read_folios_async(mdev.m_fscache->cookie, > + rreq, mdev.m_pa + (pos - map.m_la)); > if (!ret) { > - erofs_fscache_unlock_folios(rac, count); > ret = count; > } > } while (ret > 0 && ((done += ret) < len)); -- Thanks, Jeffle
