From: Franck Bui-Huu <vagabon.xyz@gmail.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org
Subject: [PATCH] do_mpage_readpage(): remove first_logical_block parameter
Date: Tue, 25 Nov 2008 21:29:11 +0100 [thread overview]
Message-ID: <492C6017.2070307@gmail.com> (raw)
From: Franck Bui-Huu <fbuihuu@gmail.com>
This patch makes this function more readable IMHO by getting rid of
its parameter 'first_logical_block'.
This parameter was previously used to remember the original block
number in the file (ie the page index) that 'map_bh' was pointing
at first. Indeed this was needed since 'map_bh->page' was modified
even if 'map_bh' maps the whole current page (IOW when get_block()
is not used). This had the bad effect to make the state of 'map_bh'
inconsistent too.
This patch changes 'map_bh->page' only when get_block() is called
hence removes the need of the 'first_logical_block' argument.
The value stored in 'logical_block_parameter' is now recalculated
each time do_mpage_readpage() but it shouldn't matter since the
operation is trivial.
Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
Andrew,
I made this when trying to understand this function. IMHO it
makes the code more readable.
I still don't know why 'map_bh->page' was setup every time
do_mpage_readpage() is called, but I'm just discovering this
code, so...
Franck
fs/mpage.c | 57 ++++++++++++++++++++++++++++-----------------------------
1 files changed, 28 insertions(+), 29 deletions(-)
diff --git a/fs/mpage.c b/fs/mpage.c
index cf05ca7..da4d66f 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -168,7 +168,7 @@ map_buffer_to_page(struct page *page, struct buffer_head *bh, int page_block)
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)
+ get_block_t get_block)
{
struct inode *inode = page->mapping->host;
const unsigned blkbits = inode->i_blkbits;
@@ -184,7 +184,7 @@ do_mpage_readpage(struct bio *bio, struct page *page, unsigned nr_pages,
int length;
int fully_mapped = 1;
unsigned nblocks;
- unsigned relative_block;
+ unsigned i;
if (page_has_buffers(page))
goto confused;
@@ -199,40 +199,42 @@ 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;
- unsigned last = nblocks - map_offset;
-
- for (relative_block = 0; ; relative_block++) {
- if (relative_block == last) {
- clear_buffer_mapped(map_bh);
- break;
+ if (buffer_mapped(map_bh)) {
+ sector_t map_bk = map_bh->b_page->index << \
+ (PAGE_CACHE_SHIFT - blkbits);
+
+ nblocks = map_bh->b_size >> blkbits;
+ if (block_in_file > map_bk && block_in_file < map_bk + nblocks) {
+ unsigned map_offset = block_in_file - map_bk;
+ sector_t map_blocknr = map_bh->b_blocknr;
+
+ for (i = map_offset; ; i++) {
+ if (i == nblocks) {
+ clear_buffer_mapped(map_bh);
+ break;
+ }
+ if (page_block == blocks_per_page)
+ break;
+ blocks[page_block] = map_blocknr + i;
+ page_block++;
+ block_in_file++;
}
- if (page_block == blocks_per_page)
- break;
- blocks[page_block] = map_bh->b_blocknr + map_offset +
- relative_block;
- page_block++;
- block_in_file++;
+ bdev = map_bh->b_bdev;
}
- bdev = map_bh->b_bdev;
}
/*
* Then do more get_blocks calls until we are done with this page.
*/
- map_bh->b_page = page;
while (page_block < blocks_per_page) {
map_bh->b_state = 0;
map_bh->b_size = 0;
if (block_in_file < last_block) {
+ map_bh->b_page = page;
map_bh->b_size = (last_block-block_in_file) << blkbits;
if (get_block(inode, block_in_file, map_bh, 0))
goto confused;
- *first_logical_block = block_in_file;
}
if (!buffer_mapped(map_bh)) {
@@ -254,7 +256,7 @@ do_mpage_readpage(struct bio *bio, struct page *page, unsigned nr_pages,
map_buffer_to_page(page, map_bh, page_block);
goto confused;
}
-
+
if (first_hole != blocks_per_page)
goto confused; /* hole -> non-hole */
@@ -262,13 +264,13 @@ do_mpage_readpage(struct bio *bio, struct page *page, unsigned nr_pages,
if (page_block && blocks[page_block-1] != map_bh->b_blocknr-1)
goto confused;
nblocks = map_bh->b_size >> blkbits;
- for (relative_block = 0; ; relative_block++) {
- if (relative_block == nblocks) {
+ for (i = 0; ; i++) {
+ if (i == nblocks) {
clear_buffer_mapped(map_bh);
break;
} else if (page_block == blocks_per_page)
break;
- blocks[page_block] = map_bh->b_blocknr+relative_block;
+ blocks[page_block] = map_bh->b_blocknr + i;
page_block++;
block_in_file++;
}
@@ -375,7 +377,6 @@ mpage_readpages(struct address_space *mapping, struct list_head *pages,
unsigned page_idx;
sector_t last_block_in_bio = 0;
struct buffer_head map_bh;
- unsigned long first_logical_block = 0;
clear_buffer_mapped(&map_bh);
for (page_idx = 0; page_idx < nr_pages; page_idx++) {
@@ -388,7 +389,6 @@ mpage_readpages(struct address_space *mapping, struct list_head *pages,
bio = do_mpage_readpage(bio, page,
nr_pages - page_idx,
&last_block_in_bio, &map_bh,
- &first_logical_block,
get_block);
}
page_cache_release(page);
@@ -408,11 +408,10 @@ 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;
clear_buffer_mapped(&map_bh);
bio = do_mpage_readpage(bio, page, 1, &last_block_in_bio,
- &map_bh, &first_logical_block, get_block);
+ &map_bh, get_block);
if (bio)
mpage_bio_submit(READ, bio);
return 0;
--
1.6.0.2.GIT
next reply other threads:[~2008-11-25 20:29 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-11-25 20:29 Franck Bui-Huu [this message]
2008-11-26 0:48 ` [PATCH] do_mpage_readpage(): remove first_logical_block parameter Andrew Morton
2008-11-26 8:03 ` Franck Bui-Huu
2008-11-26 20:04 ` [PATCH take2] " Franck Bui-Huu
2008-11-29 9:23 ` Andrew Morton
2008-11-30 10:48 ` Franck Bui-Huu
2008-11-30 13:53 ` Franck Bui-Huu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=492C6017.2070307@gmail.com \
--to=vagabon.xyz@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=linux-fsdevel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).