* [PATCHSET v2 0/4] Submit ->readpages() IO as read-ahead
@ 2018-05-29 22:17 Jens Axboe
2018-05-29 22:17 ` [PATCH 1/4] mpage: add argument structure for do_mpage_readpage() Jens Axboe
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Jens Axboe @ 2018-05-29 22:17 UTC (permalink / raw)
To: linux-fsdevel; +Cc: viro, akpm
The only caller of ->readpages() is from read-ahead, yet we don't
submit IO flagged with REQ_RAHEAD. This means we don't see it in
blktrace, for instance, which is a shame. We already make assumptions
about ->readpages() just being for read-ahead in the mpage
implementation, using readahead_gfp_mask(mapping) as out GFP mask of
choice.
This small series fixes up mpage_readpages() to submit with
REQ_RAHEAD, which takes care of file systems using mpage_readpages().
The first patch is a prep patch, that makes do_mpage_readpage() take
an argument structure.
Changes since v1:
- Fix ext4_mpage_readpages() also being used for regular reads
- Add prep patch with struct arguments for do_mpage_readpage()
fs/btrfs/extent_io.c | 2 +-
fs/ext4/ext4.h | 2 +-
fs/ext4/inode.c | 5 +-
fs/ext4/readpage.c | 5 +-
fs/mpage.c | 109 ++++++++++++++++++++++---------------------
5 files changed, 65 insertions(+), 58 deletions(-)
--
Jens Axboe
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/4] mpage: add argument structure for do_mpage_readpage() 2018-05-29 22:17 [PATCHSET v2 0/4] Submit ->readpages() IO as read-ahead Jens Axboe @ 2018-05-29 22:17 ` Jens Axboe 2018-05-29 22:17 ` [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-05-29 22:17 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. Not 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.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead 2018-05-29 22:17 [PATCHSET v2 0/4] Submit ->readpages() IO as read-ahead Jens Axboe 2018-05-29 22:17 ` [PATCH 1/4] mpage: add argument structure for do_mpage_readpage() Jens Axboe @ 2018-05-29 22:17 ` Jens Axboe 2018-05-29 22:17 ` [PATCH 3/4] btrfs: readpages() " Jens Axboe 2018-05-29 22:17 ` [PATCH 4/4] ext4: " Jens Axboe 3 siblings, 0 replies; 13+ messages in thread From: Jens Axboe @ 2018-05-29 22:17 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 it's ->readpages() implementation will now get this right. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- fs/mpage.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/fs/mpage.c b/fs/mpage.c index 0ecac5c410f4..a6344996f924 100644 --- a/fs/mpage.c +++ b/fs/mpage.c @@ -142,6 +142,7 @@ struct mpage_readpage_args { unsigned long first_logical_block; get_block_t *get_block; gfp_t gfp; + bool is_readahead; }; /* @@ -170,6 +171,7 @@ 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 = args->is_readahead ? REQ_RAHEAD : 0; unsigned nblocks; unsigned relative_block; @@ -283,7 +285,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) { @@ -300,7 +302,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) 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 +310,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 +318,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 @@ -375,6 +377,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; @@ -394,7 +397,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); -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/4] btrfs: readpages() should submit IO as read-ahead 2018-05-29 22:17 [PATCHSET v2 0/4] Submit ->readpages() IO as read-ahead Jens Axboe 2018-05-29 22:17 ` [PATCH 1/4] mpage: add argument structure for do_mpage_readpage() Jens Axboe 2018-05-29 22:17 ` [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead Jens Axboe @ 2018-05-29 22:17 ` Jens Axboe 2018-05-29 22:17 ` [PATCH 4/4] ext4: " Jens Axboe 3 siblings, 0 replies; 13+ messages in thread From: Jens Axboe @ 2018-05-29 22:17 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 e99b329002cf..32034bc1e2b9 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -3119,7 +3119,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.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/4] ext4: readpages() should submit IO as read-ahead 2018-05-29 22:17 [PATCHSET v2 0/4] Submit ->readpages() IO as read-ahead Jens Axboe ` (2 preceding siblings ...) 2018-05-29 22:17 ` [PATCH 3/4] btrfs: readpages() " Jens Axboe @ 2018-05-29 22:17 ` Jens Axboe 3 siblings, 0 replies; 13+ messages in thread From: Jens Axboe @ 2018-05-29 22:17 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 a42e71203e53..62df0322b567 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -3052,7 +3052,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 1e50c5efae67..2bb50d184bf5 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 9ffa6fad18db..28c857d3b309 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.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCHSET v3 0/4] Submit ->readpages() IO as read-ahead @ 2018-05-30 14:42 Jens Axboe 2018-05-30 14:42 ` [PATCH 2/4] mpage: mpage_readpages() should submit " Jens Axboe 0 siblings, 1 reply; 13+ messages in thread From: Jens Axboe @ 2018-05-30 14:42 UTC (permalink / raw) To: linux-fsdevel; +Cc: viro, akpm The only caller of ->readpages() is from read-ahead, yet we don't submit IO flagged with REQ_RAHEAD. This means we don't see it in blktrace, for instance, which is a shame. We already make assumptions about ->readpages() just being for read-ahead in the mpage implementation, using readahead_gfp_mask(mapping) as out GFP mask of choice. This small series fixes up mpage_readpages() to submit with REQ_RAHEAD, which takes care of file systems using mpage_readpages(). The first patch is a prep patch, that makes do_mpage_readpage() take an argument structure. Changes since v2: - Get rid of 'gfp' passing once we have is_readahead - Pack struct better, makes it 8 bytes smaller. Changes since v1: - Fix ext4_mpage_readpages() also being used for regular reads - Add prep patch with struct arguments for do_mpage_readpage() fs/btrfs/extent_io.c | 2 +- fs/ext4/ext4.h | 2 +- fs/ext4/inode.c | 5 +- fs/ext4/readpage.c | 5 +- fs/mpage.c | 115 ++++++++++++++++++++++++------------------- 5 files changed, 71 insertions(+), 58 deletions(-) ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead 2018-05-30 14:42 [PATCHSET v3 0/4] Submit ->readpages() " Jens Axboe @ 2018-05-30 14:42 ` Jens Axboe 0 siblings, 0 replies; 13+ messages in thread From: Jens Axboe @ 2018-05-30 14:42 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 it's ->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. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- fs/mpage.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) 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); -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 0/4] Submit ->readpages() IO as read-ahead @ 2018-06-21 1:07 Jens Axboe 2018-06-21 1:07 ` [PATCH 2/4] mpage: mpage_readpages() should submit " Jens Axboe 0 siblings, 1 reply; 13+ messages in thread From: Jens Axboe @ 2018-06-21 1:07 UTC (permalink / raw) To: linux-fsdevel; +Cc: viro, akpm The only caller of ->readpages() is from read-ahead, yet we don't submit IO flagged with REQ_RAHEAD. This means we don't see it in blktrace, for instance, which is a shame. Additionally, it's preventing further functional changes in the block layer for deadling with read-ahead more intelligently. We already make assumptions about ->readpages() just being for read-ahead in the mpage implementation, using readahead_gfp_mask(mapping) as out GFP mask of choice. This small series fixes up mpage_readpages() to submit with REQ_RAHEAD, which takes care of file systems using mpage_readpages(). The first patch is a prep patch, that makes do_mpage_readpage() take an argument structure. Changes since v3: - Add comments on ->readpages() purely being read-ahead - Rebase on current -git Changes since v2: - Get rid of 'gfp' passing once we have is_readahead - Pack struct better, makes it 8 bytes smaller. Changes since v1: - Fix ext4_mpage_readpages() also being used for regular reads - Add prep patch with struct arguments for do_mpage_readpage() fs/btrfs/extent_io.c | 2 +- fs/ext4/ext4.h | 2 +- fs/ext4/inode.c | 5 +- fs/ext4/readpage.c | 5 +- fs/f2fs/data.c | 5 ++ fs/mpage.c | 115 ++++++++++++++++++++++++------------------- include/linux/fs.h | 4 ++ 7 files changed, 80 insertions(+), 58 deletions(-) -- Jens Axboe ^ permalink raw reply [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() " Jens Axboe @ 2018-06-21 1:07 ` Jens Axboe 2018-06-21 17:27 ` Randy Dunlap 2018-06-21 18:47 ` Jaegeuk Kim 0 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 " 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 " 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
end of thread, other threads:[~2018-06-21 19:32 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-05-29 22:17 [PATCHSET v2 0/4] Submit ->readpages() IO as read-ahead Jens Axboe 2018-05-29 22:17 ` [PATCH 1/4] mpage: add argument structure for do_mpage_readpage() Jens Axboe 2018-05-29 22:17 ` [PATCH 2/4] mpage: mpage_readpages() should submit IO as read-ahead Jens Axboe 2018-05-29 22:17 ` [PATCH 3/4] btrfs: readpages() " Jens Axboe 2018-05-29 22:17 ` [PATCH 4/4] ext4: " Jens Axboe -- strict thread matches above, loose matches on Subject: below -- 2018-05-30 14:42 [PATCHSET v3 0/4] Submit ->readpages() " Jens Axboe 2018-05-30 14:42 ` [PATCH 2/4] mpage: mpage_readpages() should submit " Jens Axboe 2018-06-21 1:07 [PATCH v4 0/4] Submit ->readpages() " Jens Axboe 2018-06-21 1:07 ` [PATCH 2/4] mpage: mpage_readpages() should submit " Jens Axboe 2018-06-21 17:27 ` Randy Dunlap 2018-06-21 17:29 ` Jens Axboe 2018-06-21 18:47 ` Jaegeuk Kim 2018-06-21 19:18 ` Jens Axboe 2018-06-21 19:32 ` Jaegeuk Kim 2018-06-21 19:32 ` Jens Axboe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.