Re: [PATCH] Introduce a method to catch mmap_region (was: Recent kernel "mount" slow)

Linus Torvalds
2012-11-29T06:16:45Z

[-- Attachment #1: Type: text/plain, Size: 1202 bytes --]

On Wed, Nov 28, 2012 at 6:58 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> But the fact that the code wants to do things like
>
> block = (sector_t)page->index << (PAGE_CACHE_SHIFT - bbits);
>
> seriously seems to be the main thing that keeps us using
> 'inode->i_blkbits'. Calculating bbits from bh->b_size is just costly
> enough to hurt (not everywhere, but on some machines).
>
> Very annoying.

Hmm. Here's a patch that does that anyway. I'm not 100% happy with the
whole ilog2 thing, but at the same time, in other cases it actually
seems to improve code generation (ie gets rid of the whole unnecessary
two dereferences through page->mapping->host just to get the block
size, when we have it in the buffer-head that we have to touch
*anyway*).

Comments? Again, untested.

And I notice that Al Viro hasn't been cc'd, which is sad, since he's
been involved in much of fs/block_dev.c.

Al - this is an independent patch to fs/buffer.c to make
fs/block_dev.c able to change the block size of a block device while
there is IO in progress that may still use the old block size. The
discussion has been on fsdevel and lkml, but you may have missed it...

Linus

[-- Attachment #2: patch.diff --]
[-- Type: application/octet-stream, Size: 5068 bytes --]

fs/buffer.c | 60 +++++++++++++++++++++++++++++++++++++++---------------------
1 file changed, 39 insertions(+), 21 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index b5f044283edb..287bc6f2bd51 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1553,6 +1553,19 @@ void unmap_underlying_metadata(struct block_device *bdev, sector_t block)
EXPORT_SYMBOL(unmap_underlying_metadata);

/*
+ * Size is a power-of-two in the range 512..PAGE_SIZE,
+ * and the case we care about most is PAGE_SIZE.
+ *
+ * So this *could* possibly be written with those
+ * constraints in mind (relevant mostly if some
+ * architecture has a slow bit-scan instruction)
+ */
+static inline int block_size_bits(unsigned int blocksize)
+{
+ return ilog2(blocksize);
+}
+
+/*
* NOTE! All mapped/uptodate combinations are valid:
*
* Mapped Uptodate Meaning
@@ -1589,17 +1602,15 @@ static int __block_write_full_page(struct inode *inode, struct page *page,
sector_t block;
sector_t last_block;
struct buffer_head *bh, *head;
- const unsigned blocksize = 1 << inode->i_blkbits;
+ unsigned int blocksize, bbits;
int nr_underway = 0;
int write_op = (wbc->sync_mode == WB_SYNC_ALL ?
WRITE_SYNC : WRITE);

BUG_ON(!PageLocked(page));

- last_block = (i_size_read(inode) - 1) >> inode->i_blkbits;
-
if (!page_has_buffers(page)) {
- create_empty_buffers(page, blocksize,
+ create_empty_buffers(page, 1 << inode->i_blkbits,
(1 << BH_Dirty)|(1 << BH_Uptodate));
}

@@ -1613,9 +1624,13 @@ static int __block_write_full_page(struct inode *inode, struct page *page,
* handle that here by just cleaning them.
*/

- block = (sector_t)page->index << (PAGE_CACHE_SHIFT - inode->i_blkbits);
head = page_buffers(page);
bh = head;
+ blocksize = bh->b_size;
+ bbits = block_size_bits(blocksize);
+
+ block = (sector_t)page->index << (PAGE_CACHE_SHIFT - bbits);
+ last_block = (i_size_read(inode) - 1) >> bbits;

/*
* Get all the dirty buffers mapped to disk addresses and
@@ -1806,12 +1821,12 @@ int __block_write_begin(struct page *page, loff_t pos, unsigned len,
BUG_ON(to > PAGE_CACHE_SIZE);
BUG_ON(from > to);

- blocksize = 1 << inode->i_blkbits;
if (!page_has_buffers(page))
- create_empty_buffers(page, blocksize, 0);
+ create_empty_buffers(page, 1 << inode->i_blkbits, 0);
head = page_buffers(page);
+ blocksize = head->b_size;
+ bbits = block_size_bits(blocksize);

- bbits = inode->i_blkbits;
block = (sector_t)page->index << (PAGE_CACHE_SHIFT - bbits);

for(bh = head, block_start = 0; bh != head || !block_start;
@@ -1881,11 +1896,11 @@ static int __block_commit_write(struct inode *inode, struct page *page,
unsigned blocksize;
struct buffer_head *bh, *head;

- blocksize = 1 << inode->i_blkbits;
+ bh = head = page_buffers(page);
+ blocksize = bh->b_size;

- for(bh = head = page_buffers(page), block_start = 0;
- bh != head || !block_start;
- block_start=block_end, bh = bh->b_this_page) {
+ block_start = 0;
+ do {
block_end = block_start + blocksize;
if (block_end <= from || block_start >= to) {
if (!buffer_uptodate(bh))
@@ -1895,7 +1910,10 @@ static int __block_commit_write(struct inode *inode, struct page *page,
mark_buffer_dirty(bh);
}
clear_buffer_new(bh);
- }
+
+ block_start = block_end;
+ bh = bh->b_this_page;
+ } while (bh != head);

/*
* If this is a partial write which happened to make all buffers
@@ -2020,7 +2038,6 @@ EXPORT_SYMBOL(generic_write_end);
int block_is_partially_uptodate(struct page *page, read_descriptor_t *desc,
unsigned long from)
{
- struct inode *inode = page->mapping->host;
unsigned block_start, block_end, blocksize;
unsigned to;
struct buffer_head *bh, *head;
@@ -2029,13 +2046,13 @@ int block_is_partially_uptodate(struct page *page, read_descriptor_t *desc,
if (!page_has_buffers(page))
return 0;

- blocksize = 1 << inode->i_blkbits;
+ head = page_buffers(page);
+ blocksize = head->b_size;
to = min_t(unsigned, PAGE_CACHE_SIZE - from, desc->count);
to = from + to;
if (from < blocksize && to > PAGE_CACHE_SIZE - blocksize)
return 0;

- head = page_buffers(page);
bh = head;
block_start = 0;
do {
@@ -2068,18 +2085,19 @@ int block_read_full_page(struct page *page, get_block_t *get_block)
struct inode *inode = page->mapping->host;
sector_t iblock, lblock;
struct buffer_head *bh, *head, *arr[MAX_BUF_PER_PAGE];
- unsigned int blocksize;
+ unsigned int blocksize, bbits;
int nr, i;
int fully_mapped = 1;

BUG_ON(!PageLocked(page));
- blocksize = 1 << inode->i_blkbits;
if (!page_has_buffers(page))
- create_empty_buffers(page, blocksize, 0);
+ create_empty_buffers(page, 1 << inode->i_blkbits, 0);
head = page_buffers(page);
+ blocksize = head->b_size;
+ bbits = block_size_bits(blocksize);

- iblock = (sector_t)page->index << (PAGE_CACHE_SHIFT - inode->i_blkbits);
- lblock = (i_size_read(inode)+blocksize-1) >> inode->i_blkbits;
+ iblock = (sector_t)page->index << (PAGE_CACHE_SHIFT - bbits);
+ lblock = (i_size_read(inode)+blocksize-1) >> bbits;
bh = head;
nr = 0;
i = 0;