* [PATCH 1/4] mpage: add argument structure for do_mpage_readpage()
2018-06-21 1:07 [PATCH v4 0/4] Submit ->readpages() IO as read-ahead Jens Axboe
@ 2018-06-21 1:07 ` Jens Axboe
2018-06-21 1:07 ` [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead Jens Axboe
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: Jens Axboe @ 2018-06-21 1:07 UTC (permalink / raw)
To: linux-fsdevel; +Cc: viro, akpm, Jens Axboe
We're currently passing 8 arguments to this function, clean it up a
bit by packing the arguments in an args structure we pass to it.
No intentional functional changes in this patch.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/mpage.c | 106 +++++++++++++++++++++++++++--------------------------
1 file changed, 54 insertions(+), 52 deletions(-)
diff --git a/fs/mpage.c b/fs/mpage.c
index b7e7f570733a..0ecac5c410f4 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -133,6 +133,17 @@ map_buffer_to_page(struct page *page, struct buffer_head *bh, int page_block)
} while (page_bh != head);
}
+struct mpage_readpage_args {
+ struct bio *bio;
+ struct page *page;
+ unsigned nr_pages;
+ sector_t last_block_in_bio;
+ struct buffer_head map_bh;
+ unsigned long first_logical_block;
+ get_block_t *get_block;
+ gfp_t gfp;
+};
+
/*
* This is the worker routine which does all the work of mapping the disk
* blocks and constructs largest possible bios, submits them for IO if the
@@ -142,16 +153,14 @@ map_buffer_to_page(struct page *page, struct buffer_head *bh, int page_block)
* represent the validity of its disk mapping and to decide when to do the next
* get_block() call.
*/
-static struct bio *
-do_mpage_readpage(struct bio *bio, struct page *page, unsigned nr_pages,
- sector_t *last_block_in_bio, struct buffer_head *map_bh,
- unsigned long *first_logical_block, get_block_t get_block,
- gfp_t gfp)
+static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
{
+ struct page *page = args->page;
struct inode *inode = page->mapping->host;
const unsigned blkbits = inode->i_blkbits;
const unsigned blocks_per_page = PAGE_SIZE >> blkbits;
const unsigned blocksize = 1 << blkbits;
+ struct buffer_head *map_bh = &args->map_bh;
sector_t block_in_file;
sector_t last_block;
sector_t last_block_in_file;
@@ -168,7 +177,7 @@ do_mpage_readpage(struct bio *bio, struct page *page, unsigned nr_pages,
goto confused;
block_in_file = (sector_t)page->index << (PAGE_SHIFT - blkbits);
- last_block = block_in_file + nr_pages * blocks_per_page;
+ last_block = block_in_file + args->nr_pages * blocks_per_page;
last_block_in_file = (i_size_read(inode) + blocksize - 1) >> blkbits;
if (last_block > last_block_in_file)
last_block = last_block_in_file;
@@ -178,9 +187,9 @@ do_mpage_readpage(struct bio *bio, struct page *page, unsigned nr_pages,
* Map blocks using the result from the previous get_blocks call first.
*/
nblocks = map_bh->b_size >> blkbits;
- if (buffer_mapped(map_bh) && block_in_file > *first_logical_block &&
- block_in_file < (*first_logical_block + nblocks)) {
- unsigned map_offset = block_in_file - *first_logical_block;
+ if (buffer_mapped(map_bh) && block_in_file > args->first_logical_block &&
+ block_in_file < (args->first_logical_block + nblocks)) {
+ unsigned map_offset = block_in_file - args->first_logical_block;
unsigned last = nblocks - map_offset;
for (relative_block = 0; ; relative_block++) {
@@ -208,9 +217,9 @@ do_mpage_readpage(struct bio *bio, struct page *page, unsigned nr_pages,
if (block_in_file < last_block) {
map_bh->b_size = (last_block-block_in_file) << blkbits;
- if (get_block(inode, block_in_file, map_bh, 0))
+ if (args->get_block(inode, block_in_file, map_bh, 0))
goto confused;
- *first_logical_block = block_in_file;
+ args->first_logical_block = block_in_file;
}
if (!buffer_mapped(map_bh)) {
@@ -273,43 +282,43 @@ do_mpage_readpage(struct bio *bio, struct page *page, unsigned nr_pages,
/*
* This page will go to BIO. Do we need to send this BIO off first?
*/
- if (bio && (*last_block_in_bio != blocks[0] - 1))
- bio = mpage_bio_submit(REQ_OP_READ, 0, bio);
+ if (args->bio && (args->last_block_in_bio != blocks[0] - 1))
+ args->bio = mpage_bio_submit(REQ_OP_READ, 0, args->bio);
alloc_new:
- if (bio == NULL) {
+ if (args->bio == NULL) {
if (first_hole == blocks_per_page) {
if (!bdev_read_page(bdev, blocks[0] << (blkbits - 9),
page))
goto out;
}
- bio = mpage_alloc(bdev, blocks[0] << (blkbits - 9),
- min_t(int, nr_pages, BIO_MAX_PAGES), gfp);
- if (bio == NULL)
+ args->bio = mpage_alloc(bdev, blocks[0] << (blkbits - 9),
+ min_t(int, args->nr_pages, BIO_MAX_PAGES), args->gfp);
+ if (args->bio == NULL)
goto confused;
}
length = first_hole << blkbits;
- if (bio_add_page(bio, page, length, 0) < length) {
- bio = mpage_bio_submit(REQ_OP_READ, 0, bio);
+ if (bio_add_page(args->bio, page, length, 0) < length) {
+ args->bio = mpage_bio_submit(REQ_OP_READ, 0, args->bio);
goto alloc_new;
}
- relative_block = block_in_file - *first_logical_block;
+ relative_block = block_in_file - args->first_logical_block;
nblocks = map_bh->b_size >> blkbits;
if ((buffer_boundary(map_bh) && relative_block == nblocks) ||
(first_hole != blocks_per_page))
- bio = mpage_bio_submit(REQ_OP_READ, 0, bio);
+ args->bio = mpage_bio_submit(REQ_OP_READ, 0, args->bio);
else
- *last_block_in_bio = blocks[blocks_per_page - 1];
+ args->last_block_in_bio = blocks[blocks_per_page - 1];
out:
- return bio;
+ return args->bio;
confused:
- if (bio)
- bio = mpage_bio_submit(REQ_OP_READ, 0, bio);
+ if (args->bio)
+ args->bio = mpage_bio_submit(REQ_OP_READ, 0, args->bio);
if (!PageUptodate(page))
- block_read_full_page(page, get_block);
+ block_read_full_page(page, args->get_block);
else
unlock_page(page);
goto out;
@@ -363,15 +372,12 @@ int
mpage_readpages(struct address_space *mapping, struct list_head *pages,
unsigned nr_pages, get_block_t get_block)
{
- struct bio *bio = NULL;
+ struct mpage_readpage_args args = {
+ .get_block = get_block,
+ .gfp = readahead_gfp_mask(mapping),
+ };
unsigned page_idx;
- sector_t last_block_in_bio = 0;
- struct buffer_head map_bh;
- unsigned long first_logical_block = 0;
- gfp_t gfp = readahead_gfp_mask(mapping);
- map_bh.b_state = 0;
- map_bh.b_size = 0;
for (page_idx = 0; page_idx < nr_pages; page_idx++) {
struct page *page = lru_to_page(pages);
@@ -379,18 +385,16 @@ mpage_readpages(struct address_space *mapping, struct list_head *pages,
list_del(&page->lru);
if (!add_to_page_cache_lru(page, mapping,
page->index,
- gfp)) {
- bio = do_mpage_readpage(bio, page,
- nr_pages - page_idx,
- &last_block_in_bio, &map_bh,
- &first_logical_block,
- get_block, gfp);
+ args.gfp)) {
+ args.page = page;
+ args.nr_pages = nr_pages - page_idx;
+ args.bio = do_mpage_readpage(&args);
}
put_page(page);
}
BUG_ON(!list_empty(pages));
- if (bio)
- mpage_bio_submit(REQ_OP_READ, 0, bio);
+ if (args.bio)
+ mpage_bio_submit(REQ_OP_READ, 0, args.bio);
return 0;
}
EXPORT_SYMBOL(mpage_readpages);
@@ -400,18 +404,16 @@ EXPORT_SYMBOL(mpage_readpages);
*/
int mpage_readpage(struct page *page, get_block_t get_block)
{
- struct bio *bio = NULL;
- sector_t last_block_in_bio = 0;
- struct buffer_head map_bh;
- unsigned long first_logical_block = 0;
- gfp_t gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL);
+ struct mpage_readpage_args args = {
+ .page = page,
+ .nr_pages = 1,
+ .get_block = get_block,
+ .gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL),
+ };
- map_bh.b_state = 0;
- map_bh.b_size = 0;
- bio = do_mpage_readpage(bio, page, 1, &last_block_in_bio,
- &map_bh, &first_logical_block, get_block, gfp);
- if (bio)
- mpage_bio_submit(REQ_OP_READ, 0, bio);
+ args.bio = do_mpage_readpage(&args);
+ if (args.bio)
+ mpage_bio_submit(REQ_OP_READ, 0, args.bio);
return 0;
}
EXPORT_SYMBOL(mpage_readpage);
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead
2018-06-21 1:07 [PATCH v4 0/4] Submit ->readpages() IO as read-ahead Jens Axboe
2018-06-21 1:07 ` [PATCH 1/4] mpage: add argument structure for do_mpage_readpage() Jens Axboe
@ 2018-06-21 1:07 ` Jens Axboe
2018-06-21 17:27 ` Randy Dunlap
2018-06-21 18:47 ` Jaegeuk Kim
2018-06-21 1:07 ` [PATCH 3/4] btrfs: readpages() " Jens Axboe
2018-06-21 1:07 ` [PATCH 4/4] ext4: " Jens Axboe
3 siblings, 2 replies; 13+ messages in thread
From: Jens Axboe @ 2018-06-21 1:07 UTC (permalink / raw)
To: linux-fsdevel; +Cc: viro, akpm, Jens Axboe
a_ops->readpages() is only ever used for read-ahead, yet we don't
flag the IO being submitted as such. Fix that up. Any file system
that uses mpage_readpages() as its ->readpages() implementation
will now get this right.
Since we're passing in whether the IO is read-ahead or not, we
don't need to pass in the 'gfp' separately, as it is dependent
on the IO being read-ahead. Kill off that member.
Add some documentation notes on ->readpages() being purely for
read-ahead.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/f2fs/data.c | 5 +++++
fs/mpage.c | 29 +++++++++++++++++++----------
include/linux/fs.h | 4 ++++
3 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 8f931d699287..b7c9b58acf3e 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1421,6 +1421,11 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
/*
* This function was originally taken from fs/mpage.c, and customized for f2fs.
* Major change was from block_size == page_size in f2fs by default.
+ *
+ * Note that the aops->readpages() function is ONLY used for read-ahead. If
+ * this function ever deviates from doing just read-ahead, it should either
+ * use ->readpage() or do the necessary surgery to decouple ->readpages()
+ * readom read-ahead.
*/
static int f2fs_mpage_readpages(struct address_space *mapping,
struct list_head *pages, struct page *page,
diff --git a/fs/mpage.c b/fs/mpage.c
index 0ecac5c410f4..b0f9de977526 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -137,11 +137,11 @@ struct mpage_readpage_args {
struct bio *bio;
struct page *page;
unsigned nr_pages;
+ bool is_readahead;
sector_t last_block_in_bio;
struct buffer_head map_bh;
unsigned long first_logical_block;
get_block_t *get_block;
- gfp_t gfp;
};
/*
@@ -170,8 +170,18 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
struct block_device *bdev = NULL;
int length;
int fully_mapped = 1;
+ int op_flags;
unsigned nblocks;
unsigned relative_block;
+ gfp_t gfp;
+
+ if (args->is_readahead) {
+ op_flags = REQ_RAHEAD;
+ gfp = readahead_gfp_mask(page->mapping);
+ } else {
+ op_flags = 0;
+ gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL);
+ }
if (page_has_buffers(page))
goto confused;
@@ -283,7 +293,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
* This page will go to BIO. Do we need to send this BIO off first?
*/
if (args->bio && (args->last_block_in_bio != blocks[0] - 1))
- args->bio = mpage_bio_submit(REQ_OP_READ, 0, args->bio);
+ args->bio = mpage_bio_submit(REQ_OP_READ, op_flags, args->bio);
alloc_new:
if (args->bio == NULL) {
@@ -293,14 +303,14 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
goto out;
}
args->bio = mpage_alloc(bdev, blocks[0] << (blkbits - 9),
- min_t(int, args->nr_pages, BIO_MAX_PAGES), args->gfp);
+ min_t(int, args->nr_pages, BIO_MAX_PAGES), gfp);
if (args->bio == NULL)
goto confused;
}
length = first_hole << blkbits;
if (bio_add_page(args->bio, page, length, 0) < length) {
- args->bio = mpage_bio_submit(REQ_OP_READ, 0, args->bio);
+ args->bio = mpage_bio_submit(REQ_OP_READ, op_flags, args->bio);
goto alloc_new;
}
@@ -308,7 +318,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
nblocks = map_bh->b_size >> blkbits;
if ((buffer_boundary(map_bh) && relative_block == nblocks) ||
(first_hole != blocks_per_page))
- args->bio = mpage_bio_submit(REQ_OP_READ, 0, args->bio);
+ args->bio = mpage_bio_submit(REQ_OP_READ, op_flags, args->bio);
else
args->last_block_in_bio = blocks[blocks_per_page - 1];
out:
@@ -316,7 +326,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
confused:
if (args->bio)
- args->bio = mpage_bio_submit(REQ_OP_READ, 0, args->bio);
+ args->bio = mpage_bio_submit(REQ_OP_READ, op_flags, args->bio);
if (!PageUptodate(page))
block_read_full_page(page, args->get_block);
else
@@ -374,7 +384,7 @@ mpage_readpages(struct address_space *mapping, struct list_head *pages,
{
struct mpage_readpage_args args = {
.get_block = get_block,
- .gfp = readahead_gfp_mask(mapping),
+ .is_readahead = true,
};
unsigned page_idx;
@@ -385,7 +395,7 @@ mpage_readpages(struct address_space *mapping, struct list_head *pages,
list_del(&page->lru);
if (!add_to_page_cache_lru(page, mapping,
page->index,
- args.gfp)) {
+ readahead_gfp_mask(mapping))) {
args.page = page;
args.nr_pages = nr_pages - page_idx;
args.bio = do_mpage_readpage(&args);
@@ -394,7 +404,7 @@ mpage_readpages(struct address_space *mapping, struct list_head *pages,
}
BUG_ON(!list_empty(pages));
if (args.bio)
- mpage_bio_submit(REQ_OP_READ, 0, args.bio);
+ mpage_bio_submit(REQ_OP_READ, REQ_RAHEAD, args.bio);
return 0;
}
EXPORT_SYMBOL(mpage_readpages);
@@ -408,7 +418,6 @@ int mpage_readpage(struct page *page, get_block_t get_block)
.page = page,
.nr_pages = 1,
.get_block = get_block,
- .gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL),
};
args.bio = do_mpage_readpage(&args);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5c91108846db..b45c34a9b28d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -341,6 +341,10 @@ struct address_space_operations {
/* Set a page dirty. Return true if this dirtied it */
int (*set_page_dirty)(struct page *page);
+ /*
+ * Reads in the requested pages. Unlike ->readpage(), this is
+ * PURELY used for read-ahead!.
+ */
int (*readpages)(struct file *filp, struct address_space *mapping,
struct list_head *pages, unsigned nr_pages);
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead
2018-06-21 1:07 ` [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead Jens Axboe
@ 2018-06-21 17:27 ` Randy Dunlap
2018-06-21 17:29 ` Jens Axboe
2018-06-21 18:47 ` Jaegeuk Kim
1 sibling, 1 reply; 13+ messages in thread
From: Randy Dunlap @ 2018-06-21 17:27 UTC (permalink / raw)
To: Jens Axboe, linux-fsdevel; +Cc: viro, akpm
On 06/20/2018 06:07 PM, Jens Axboe wrote:
> a_ops->readpages() is only ever used for read-ahead, yet we don't
> flag the IO being submitted as such. Fix that up. Any file system
> that uses mpage_readpages() as its ->readpages() implementation
> will now get this right.
>
> Since we're passing in whether the IO is read-ahead or not, we
> don't need to pass in the 'gfp' separately, as it is dependent
> on the IO being read-ahead. Kill off that member.
>
> Add some documentation notes on ->readpages() being purely for
> read-ahead.
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> fs/f2fs/data.c | 5 +++++
> fs/mpage.c | 29 +++++++++++++++++++----------
> include/linux/fs.h | 4 ++++
> 3 files changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 8f931d699287..b7c9b58acf3e 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1421,6 +1421,11 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> /*
> * This function was originally taken from fs/mpage.c, and customized for f2fs.
> * Major change was from block_size == page_size in f2fs by default.
> + *
> + * Note that the aops->readpages() function is ONLY used for read-ahead. If
> + * this function ever deviates from doing just read-ahead, it should either
> + * use ->readpage() or do the necessary surgery to decouple ->readpages()
> + * readom read-ahead.
eh?
> */
--
~Randy
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead
2018-06-21 17:27 ` Randy Dunlap
@ 2018-06-21 17:29 ` Jens Axboe
0 siblings, 0 replies; 13+ messages in thread
From: Jens Axboe @ 2018-06-21 17:29 UTC (permalink / raw)
To: Randy Dunlap, linux-fsdevel; +Cc: viro, akpm
On 6/21/18 11:27 AM, Randy Dunlap wrote:
> On 06/20/2018 06:07 PM, Jens Axboe wrote:
>> a_ops->readpages() is only ever used for read-ahead, yet we don't
>> flag the IO being submitted as such. Fix that up. Any file system
>> that uses mpage_readpages() as its ->readpages() implementation
>> will now get this right.
>>
>> Since we're passing in whether the IO is read-ahead or not, we
>> don't need to pass in the 'gfp' separately, as it is dependent
>> on the IO being read-ahead. Kill off that member.
>>
>> Add some documentation notes on ->readpages() being purely for
>> read-ahead.
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>> fs/f2fs/data.c | 5 +++++
>> fs/mpage.c | 29 +++++++++++++++++++----------
>> include/linux/fs.h | 4 ++++
>> 3 files changed, 28 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index 8f931d699287..b7c9b58acf3e 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -1421,6 +1421,11 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>> /*
>> * This function was originally taken from fs/mpage.c, and customized for f2fs.
>> * Major change was from block_size == page_size in f2fs by default.
>> + *
>> + * Note that the aops->readpages() function is ONLY used for read-ahead. If
>> + * this function ever deviates from doing just read-ahead, it should either
>> + * use ->readpage() or do the necessary surgery to decouple ->readpages()
>> + * readom read-ahead.
>
> eh?
Huh weird, must be touchpad and typing. Should be 'from read-ahead'.
--
Jens Axboe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead
2018-06-21 1:07 ` [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead Jens Axboe
2018-06-21 17:27 ` Randy Dunlap
@ 2018-06-21 18:47 ` Jaegeuk Kim
2018-06-21 19:18 ` Jens Axboe
1 sibling, 1 reply; 13+ messages in thread
From: Jaegeuk Kim @ 2018-06-21 18:47 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-fsdevel, viro, akpm
On 06/20, Jens Axboe wrote:
> a_ops->readpages() is only ever used for read-ahead, yet we don't
> flag the IO being submitted as such. Fix that up. Any file system
> that uses mpage_readpages() as its ->readpages() implementation
> will now get this right.
>
> Since we're passing in whether the IO is read-ahead or not, we
> don't need to pass in the 'gfp' separately, as it is dependent
> on the IO being read-ahead. Kill off that member.
>
> Add some documentation notes on ->readpages() being purely for
> read-ahead.
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> fs/f2fs/data.c | 5 +++++
> fs/mpage.c | 29 +++++++++++++++++++----------
> include/linux/fs.h | 4 ++++
> 3 files changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 8f931d699287..b7c9b58acf3e 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1421,6 +1421,11 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> /*
> * This function was originally taken from fs/mpage.c, and customized for f2fs.
> * Major change was from block_size == page_size in f2fs by default.
> + *
> + * Note that the aops->readpages() function is ONLY used for read-ahead. If
> + * this function ever deviates from doing just read-ahead, it should either
> + * use ->readpage() or do the necessary surgery to decouple ->readpages()
> + * readom read-ahead.
> */
> static int f2fs_mpage_readpages(struct address_space *mapping,
> struct list_head *pages, struct page *page,
Hi Jens,
Could you please consider the below change to address your concern?
---
fs/f2fs/data.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 8f931d699287..c1266b231f62 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -534,7 +534,7 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
}
static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
- unsigned nr_pages)
+ unsigned nr_pages, unsigned op_flag)
{
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
struct bio *bio;
@@ -546,7 +546,7 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
return ERR_PTR(-ENOMEM);
f2fs_target_device(sbi, blkaddr, bio);
bio->bi_end_io = f2fs_read_end_io;
- bio_set_op_attrs(bio, REQ_OP_READ, 0);
+ bio_set_op_attrs(bio, REQ_OP_READ, op_flag);
if (f2fs_encrypted_file(inode))
post_read_steps |= 1 << STEP_DECRYPT;
@@ -571,7 +571,7 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
static int f2fs_submit_page_read(struct inode *inode, struct page *page,
block_t blkaddr)
{
- struct bio *bio = f2fs_grab_read_bio(inode, blkaddr, 1);
+ struct bio *bio = f2fs_grab_read_bio(inode, blkaddr, 1, 0);
if (IS_ERR(bio))
return PTR_ERR(bio);
@@ -1424,7 +1424,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
*/
static int f2fs_mpage_readpages(struct address_space *mapping,
struct list_head *pages, struct page *page,
- unsigned nr_pages)
+ unsigned nr_pages, bool is_readahead)
{
struct bio *bio = NULL;
sector_t last_block_in_bio = 0;
@@ -1514,7 +1514,8 @@ static int f2fs_mpage_readpages(struct address_space *mapping,
bio = NULL;
}
if (bio == NULL) {
- bio = f2fs_grab_read_bio(inode, block_nr, nr_pages);
+ bio = f2fs_grab_read_bio(inode, block_nr, nr_pages,
+ is_readahead ? REQ_RAHEAD : 0);
if (IS_ERR(bio)) {
bio = NULL;
goto set_error_page;
@@ -1558,7 +1559,7 @@ static int f2fs_read_data_page(struct file *file, struct page *page)
if (f2fs_has_inline_data(inode))
ret = f2fs_read_inline_data(inode, page);
if (ret == -EAGAIN)
- ret = f2fs_mpage_readpages(page->mapping, NULL, page, 1);
+ ret = f2fs_mpage_readpages(page->mapping, NULL, page, 1, false);
return ret;
}
@@ -1575,7 +1576,7 @@ static int f2fs_read_data_pages(struct file *file,
if (f2fs_has_inline_data(inode))
return 0;
- return f2fs_mpage_readpages(mapping, pages, NULL, nr_pages);
+ return f2fs_mpage_readpages(mapping, pages, NULL, nr_pages, true);
}
static int encrypt_one_page(struct f2fs_io_info *fio)
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead
2018-06-21 18:47 ` Jaegeuk Kim
@ 2018-06-21 19:18 ` Jens Axboe
2018-06-21 19:32 ` Jaegeuk Kim
0 siblings, 1 reply; 13+ messages in thread
From: Jens Axboe @ 2018-06-21 19:18 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-fsdevel, viro, akpm
On 6/21/18 12:47 PM, Jaegeuk Kim wrote:
> On 06/20, Jens Axboe wrote:
>> a_ops->readpages() is only ever used for read-ahead, yet we don't
>> flag the IO being submitted as such. Fix that up. Any file system
>> that uses mpage_readpages() as its ->readpages() implementation
>> will now get this right.
>>
>> Since we're passing in whether the IO is read-ahead or not, we
>> don't need to pass in the 'gfp' separately, as it is dependent
>> on the IO being read-ahead. Kill off that member.
>>
>> Add some documentation notes on ->readpages() being purely for
>> read-ahead.
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>> fs/f2fs/data.c | 5 +++++
>> fs/mpage.c | 29 +++++++++++++++++++----------
>> include/linux/fs.h | 4 ++++
>> 3 files changed, 28 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index 8f931d699287..b7c9b58acf3e 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -1421,6 +1421,11 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>> /*
>> * This function was originally taken from fs/mpage.c, and customized for f2fs.
>> * Major change was from block_size == page_size in f2fs by default.
>> + *
>> + * Note that the aops->readpages() function is ONLY used for read-ahead. If
>> + * this function ever deviates from doing just read-ahead, it should either
>> + * use ->readpage() or do the necessary surgery to decouple ->readpages()
>> + * readom read-ahead.
>> */
>> static int f2fs_mpage_readpages(struct address_space *mapping,
>> struct list_head *pages, struct page *page,
>
> Hi Jens,
>
> Could you please consider the below change to address your concern?
Looks good to me.
--
Jens Axboe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead
2018-06-21 19:18 ` Jens Axboe
@ 2018-06-21 19:32 ` Jaegeuk Kim
2018-06-21 19:32 ` Jens Axboe
0 siblings, 1 reply; 13+ messages in thread
From: Jaegeuk Kim @ 2018-06-21 19:32 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-fsdevel, viro, akpm
On 06/21, Jens Axboe wrote:
> On 6/21/18 12:47 PM, Jaegeuk Kim wrote:
> > On 06/20, Jens Axboe wrote:
> >> a_ops->readpages() is only ever used for read-ahead, yet we don't
> >> flag the IO being submitted as such. Fix that up. Any file system
> >> that uses mpage_readpages() as its ->readpages() implementation
> >> will now get this right.
> >>
> >> Since we're passing in whether the IO is read-ahead or not, we
> >> don't need to pass in the 'gfp' separately, as it is dependent
> >> on the IO being read-ahead. Kill off that member.
> >>
> >> Add some documentation notes on ->readpages() being purely for
> >> read-ahead.
> >>
> >> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> >> ---
> >> fs/f2fs/data.c | 5 +++++
> >> fs/mpage.c | 29 +++++++++++++++++++----------
> >> include/linux/fs.h | 4 ++++
> >> 3 files changed, 28 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> >> index 8f931d699287..b7c9b58acf3e 100644
> >> --- a/fs/f2fs/data.c
> >> +++ b/fs/f2fs/data.c
> >> @@ -1421,6 +1421,11 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> >> /*
> >> * This function was originally taken from fs/mpage.c, and customized for f2fs.
> >> * Major change was from block_size == page_size in f2fs by default.
> >> + *
> >> + * Note that the aops->readpages() function is ONLY used for read-ahead. If
> >> + * this function ever deviates from doing just read-ahead, it should either
> >> + * use ->readpage() or do the necessary surgery to decouple ->readpages()
> >> + * readom read-ahead.
> >> */
> >> static int f2fs_mpage_readpages(struct address_space *mapping,
> >> struct list_head *pages, struct page *page,
> >
> > Hi Jens,
> >
> > Could you please consider the below change to address your concern?
>
> Looks good to me.
Let me add this change in f2fs tree.
Thanks,
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead
2018-06-21 19:32 ` Jaegeuk Kim
@ 2018-06-21 19:32 ` Jens Axboe
0 siblings, 0 replies; 13+ messages in thread
From: Jens Axboe @ 2018-06-21 19:32 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-fsdevel, viro, akpm
On 6/21/18 1:32 PM, Jaegeuk Kim wrote:
> On 06/21, Jens Axboe wrote:
>> On 6/21/18 12:47 PM, Jaegeuk Kim wrote:
>>> On 06/20, Jens Axboe wrote:
>>>> a_ops->readpages() is only ever used for read-ahead, yet we don't
>>>> flag the IO being submitted as such. Fix that up. Any file system
>>>> that uses mpage_readpages() as its ->readpages() implementation
>>>> will now get this right.
>>>>
>>>> Since we're passing in whether the IO is read-ahead or not, we
>>>> don't need to pass in the 'gfp' separately, as it is dependent
>>>> on the IO being read-ahead. Kill off that member.
>>>>
>>>> Add some documentation notes on ->readpages() being purely for
>>>> read-ahead.
>>>>
>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>> ---
>>>> fs/f2fs/data.c | 5 +++++
>>>> fs/mpage.c | 29 +++++++++++++++++++----------
>>>> include/linux/fs.h | 4 ++++
>>>> 3 files changed, 28 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index 8f931d699287..b7c9b58acf3e 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -1421,6 +1421,11 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>> /*
>>>> * This function was originally taken from fs/mpage.c, and customized for f2fs.
>>>> * Major change was from block_size == page_size in f2fs by default.
>>>> + *
>>>> + * Note that the aops->readpages() function is ONLY used for read-ahead. If
>>>> + * this function ever deviates from doing just read-ahead, it should either
>>>> + * use ->readpage() or do the necessary surgery to decouple ->readpages()
>>>> + * readom read-ahead.
>>>> */
>>>> static int f2fs_mpage_readpages(struct address_space *mapping,
>>>> struct list_head *pages, struct page *page,
>>>
>>> Hi Jens,
>>>
>>> Could you please consider the below change to address your concern?
>>
>> Looks good to me.
>
> Let me add this change in f2fs tree.
Thanks!
--
Jens Axboe
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] btrfs: readpages() should submit IO as read-ahead
2018-06-21 1:07 [PATCH v4 0/4] Submit ->readpages() IO as read-ahead Jens Axboe
2018-06-21 1:07 ` [PATCH 1/4] mpage: add argument structure for do_mpage_readpage() Jens Axboe
2018-06-21 1:07 ` [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead Jens Axboe
@ 2018-06-21 1:07 ` Jens Axboe
2018-06-21 1:07 ` [PATCH 4/4] ext4: " Jens Axboe
3 siblings, 0 replies; 13+ messages in thread
From: Jens Axboe @ 2018-06-21 1:07 UTC (permalink / raw)
To: linux-fsdevel; +Cc: viro, akpm, Jens Axboe
a_ops->readpages() is only ever used for read-ahead. Ensure that we
pass this information down to the block layer.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/btrfs/extent_io.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index cce6087d6880..440e2bdc518d 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3116,7 +3116,7 @@ static inline void __do_contiguous_readpages(struct extent_io_tree *tree,
for (index = 0; index < nr_pages; index++) {
__do_readpage(tree, pages[index], btrfs_get_extent, em_cached,
- bio, 0, bio_flags, 0, prev_em_start);
+ bio, 0, bio_flags, REQ_RAHEAD, prev_em_start);
put_page(pages[index]);
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/4] ext4: readpages() should submit IO as read-ahead
2018-06-21 1:07 [PATCH v4 0/4] Submit ->readpages() IO as read-ahead Jens Axboe
` (2 preceding siblings ...)
2018-06-21 1:07 ` [PATCH 3/4] btrfs: readpages() " Jens Axboe
@ 2018-06-21 1:07 ` Jens Axboe
3 siblings, 0 replies; 13+ messages in thread
From: Jens Axboe @ 2018-06-21 1:07 UTC (permalink / raw)
To: linux-fsdevel; +Cc: viro, akpm, Jens Axboe
a_ops->readpages() is only ever used for read-ahead. Ensure that we
pass this information down to the block layer.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/ext4/ext4.h | 2 +-
fs/ext4/inode.c | 5 +++--
fs/ext4/readpage.c | 5 +++--
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 0b127853c584..e416cb0a2b8c 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3065,7 +3065,7 @@ static inline void ext4_set_de_type(struct super_block *sb,
/* readpages.c */
extern int ext4_mpage_readpages(struct address_space *mapping,
struct list_head *pages, struct page *page,
- unsigned nr_pages);
+ unsigned nr_pages, bool is_readahead);
/* symlink.c */
extern const struct inode_operations ext4_encrypted_symlink_inode_operations;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 2ea07efbe016..de1235fa1bfb 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3327,7 +3327,8 @@ static int ext4_readpage(struct file *file, struct page *page)
ret = ext4_readpage_inline(inode, page);
if (ret == -EAGAIN)
- return ext4_mpage_readpages(page->mapping, NULL, page, 1);
+ return ext4_mpage_readpages(page->mapping, NULL, page, 1,
+ false);
return ret;
}
@@ -3342,7 +3343,7 @@ ext4_readpages(struct file *file, struct address_space *mapping,
if (ext4_has_inline_data(inode))
return 0;
- return ext4_mpage_readpages(mapping, pages, NULL, nr_pages);
+ return ext4_mpage_readpages(mapping, pages, NULL, nr_pages, true);
}
static void ext4_invalidatepage(struct page *page, unsigned int offset,
diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
index 19b87a8de6ff..f461d75ac049 100644
--- a/fs/ext4/readpage.c
+++ b/fs/ext4/readpage.c
@@ -98,7 +98,7 @@ static void mpage_end_io(struct bio *bio)
int ext4_mpage_readpages(struct address_space *mapping,
struct list_head *pages, struct page *page,
- unsigned nr_pages)
+ unsigned nr_pages, bool is_readahead)
{
struct bio *bio = NULL;
sector_t last_block_in_bio = 0;
@@ -259,7 +259,8 @@ int ext4_mpage_readpages(struct address_space *mapping,
bio->bi_iter.bi_sector = blocks[0] << (blkbits - 9);
bio->bi_end_io = mpage_end_io;
bio->bi_private = ctx;
- bio_set_op_attrs(bio, REQ_OP_READ, 0);
+ bio_set_op_attrs(bio, REQ_OP_READ,
+ is_readahead ? REQ_RAHEAD : 0);
}
length = first_hole << blkbits;
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread