From: Franck Bui-Huu <fbuihuu@xxxxxxxxx>
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.
While at it, this patch does a few extra cleanups such as removing
a trailing space, moving 2 declarations of local variables closer
to the place they're used.
Signed-off-by: Franck Bui-Huu <fbuihuu@xxxxxxxxx>
---
Andrew,
I tried to address all points you raised.
The most important was the bug introduced by the first version that you
catched.
The second one was to find some good names for some locals. I also
moved their statements closer to the place they're used.
Franck
fs/mpage.c | 64 ++++++++++++++++++++++++++++++-----------------------------
1 files changed, 33 insertions(+), 31 deletions(-)
diff --git a/fs/mpage.c b/fs/mpage.c
index cf05ca7..a041a0e 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;
@@ -183,8 +183,6 @@ do_mpage_readpage(struct bio *bio, struct page *page, unsigned nr_pages,
struct block_device *bdev = NULL;
int length;
int fully_mapped = 1;
- unsigned nblocks;
- unsigned relative_block;
if (page_has_buffers(page))
goto confused;
@@ -199,40 +197,45 @@ 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)) {
+ unsigned nblocks = map_bh->b_size >> blkbits;
+ sector_t map_block = (sector_t)map_bh->b_page->index <<
+ (PAGE_CACHE_SHIFT - blkbits);
+
+ if (block_in_file > map_block &&
+ block_in_file < map_block + nblocks) {
+ unsigned blocknr_off = block_in_file - map_block;
+ sector_t map_blocknr = map_bh->b_blocknr;
+
+ for (;; blocknr_off++) {
+ if (blocknr_off == nblocks) {
+ clear_buffer_mapped(map_bh);
+ break;
+ }
+ if (page_block == blocks_per_page)
+ break;
+ blocks[page_block] = map_blocknr + blocknr_off;
+ 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) {
+ unsigned nblocks, blocknr_off = 0;
+
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,21 +257,23 @@ 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 */
/* Contiguous blocks? */
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 (;; blocknr_off++) {
+ if (blocknr_off == nblocks) {
clear_buffer_mapped(map_bh);
break;
- } else if (page_block == blocks_per_page)
+ }
+ if (page_block == blocks_per_page)
break;
- blocks[page_block] = map_bh->b_blocknr+relative_block;
+ blocks[page_block] = map_bh->b_blocknr + blocknr_off;
page_block++;
block_in_file++;
}
@@ -375,7 +380,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 +392,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 +411,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
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
