Re: [Cluster-devel] [PATCH v6 18/19] iomap: Convert from readpages to readahead

2020-02-18 Thread Dave Chinner
On Mon, Feb 17, 2020 at 10:46:12AM -0800, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" 
> 
> Use the new readahead operation in iomap.  Convert XFS and ZoneFS to
> use it.
> 
> Signed-off-by: Matthew Wilcox (Oracle) 
> ---
>  fs/iomap/buffered-io.c | 91 +++---
>  fs/iomap/trace.h   |  2 +-
>  fs/xfs/xfs_aops.c  | 13 +++---
>  fs/zonefs/super.c  |  7 ++--
>  include/linux/iomap.h  |  3 +-
>  5 files changed, 42 insertions(+), 74 deletions(-)

All pretty straight forward...

> @@ -416,6 +384,7 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, 
> loff_t length,
>   break;
>   done += ret;
>   if (offset_in_page(pos + done) == 0) {
> + readahead_next(ctx->rac);
>   if (!ctx->cur_page_in_bio)
>   unlock_page(ctx->cur_page);
>   put_page(ctx->cur_page);

Though now I look at the addition here, this might be better
restructured to mention how we handle partial page submission such as:

done += ret;

/*
 * Keep working on a partially complete page, otherwise ready
 * the ctx for the next page to be acted on.
 */
if (offset_in_page(pos + done))
continue;

if (!ctx->cur_page_in_bio)
unlock_page(ctx->cur_page);
put_page(ctx->cur_page);
ctx->cur_page = NULL;
readahead_next(ctx->rac);
}

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com




[Cluster-devel] [PATCH v6 18/19] iomap: Convert from readpages to readahead

2020-02-17 Thread Matthew Wilcox
From: "Matthew Wilcox (Oracle)" 

Use the new readahead operation in iomap.  Convert XFS and ZoneFS to
use it.

Signed-off-by: Matthew Wilcox (Oracle) 
---
 fs/iomap/buffered-io.c | 91 +++---
 fs/iomap/trace.h   |  2 +-
 fs/xfs/xfs_aops.c  | 13 +++---
 fs/zonefs/super.c  |  7 ++--
 include/linux/iomap.h  |  3 +-
 5 files changed, 42 insertions(+), 74 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 44303f370b2d..2bfcd5242264 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -214,9 +214,8 @@ iomap_read_end_io(struct bio *bio)
 struct iomap_readpage_ctx {
struct page *cur_page;
boolcur_page_in_bio;
-   boolis_readahead;
struct bio  *bio;
-   struct list_head*pages;
+   struct readahead_control *rac;
 };
 
 static void
@@ -307,11 +306,11 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, 
loff_t length, void *data,
if (ctx->bio)
submit_bio(ctx->bio);
 
-   if (ctx->is_readahead) /* same as readahead_gfp_mask */
+   if (ctx->rac) /* same as readahead_gfp_mask */
gfp |= __GFP_NORETRY | __GFP_NOWARN;
ctx->bio = bio_alloc(gfp, min(BIO_MAX_PAGES, nr_vecs));
ctx->bio->bi_opf = REQ_OP_READ;
-   if (ctx->is_readahead)
+   if (ctx->rac)
ctx->bio->bi_opf |= REQ_RAHEAD;
ctx->bio->bi_iter.bi_sector = sector;
bio_set_dev(ctx->bio, iomap->bdev);
@@ -367,36 +366,8 @@ iomap_readpage(struct page *page, const struct iomap_ops 
*ops)
 }
 EXPORT_SYMBOL_GPL(iomap_readpage);
 
-static struct page *
-iomap_next_page(struct inode *inode, struct list_head *pages, loff_t pos,
-   loff_t length, loff_t *done)
-{
-   while (!list_empty(pages)) {
-   struct page *page = lru_to_page(pages);
-
-   if (page_offset(page) >= (u64)pos + length)
-   break;
-
-   list_del(>lru);
-   if (!add_to_page_cache_lru(page, inode->i_mapping, page->index,
-   GFP_NOFS))
-   return page;
-
-   /*
-* If we already have a page in the page cache at index we are
-* done.  Upper layers don't care if it is uptodate after the
-* readpages call itself as every page gets checked again once
-* actually needed.
-*/
-   *done += PAGE_SIZE;
-   put_page(page);
-   }
-
-   return NULL;
-}
-
 static loff_t
-iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length,
+iomap_readahead_actor(struct inode *inode, loff_t pos, loff_t length,
void *data, struct iomap *iomap, struct iomap *srcmap)
 {
struct iomap_readpage_ctx *ctx = data;
@@ -404,10 +375,7 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, 
loff_t length,
 
while (done < length) {
if (!ctx->cur_page) {
-   ctx->cur_page = iomap_next_page(inode, ctx->pages,
-   pos, length, );
-   if (!ctx->cur_page)
-   break;
+   ctx->cur_page = readahead_page(ctx->rac);
ctx->cur_page_in_bio = false;
}
ret = iomap_readpage_actor(inode, pos + done, length - done,
@@ -416,6 +384,7 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, 
loff_t length,
break;
done += ret;
if (offset_in_page(pos + done) == 0) {
+   readahead_next(ctx->rac);
if (!ctx->cur_page_in_bio)
unlock_page(ctx->cur_page);
put_page(ctx->cur_page);
@@ -426,44 +395,48 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, 
loff_t length,
return done;
 }
 
-int
-iomap_readpages(struct address_space *mapping, struct list_head *pages,
-   unsigned nr_pages, const struct iomap_ops *ops)
+/**
+ * iomap_readahead - Attempt to read pages from a file.
+ * @rac: Describes the pages to be read.
+ * @ops: The operations vector for the filesystem.
+ *
+ * This function is for filesystems to call to implement their readahead
+ * address_space operation.
+ *
+ * Context: The file is pinned by the caller, and the pages to be read are
+ * all locked and have an elevated refcount.  This function will unlock
+ * the pages (once I/O has completed on them, or I/O has been determined to
+ * not be necessary).  It will also decrease the refcount once the pages
+ * have been submitted for I/O.  After this point, the page may be removed
+ * from the page cache, and should not be referenced.
+ */
+void