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. Signed-off-by: Franck Bui-Huu <fbuihuu@xxxxxxxxx> --- 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 -- 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