[PATCH] vfs: calculate block-size correctly

[PATCH] vfs: calculate block-size correctly

[Cong Meng]

I sent this patch half mount ago, but no response at all. If anything is
wrong, please tell me. Thanks a lots. This is my first time to send patch
to linux kernel.

The invocation of __getblk(bdev, block, size) will cause kernel stall if
the @size parameter is not equal to the blockr-size of @bdev, which is saved
in bdev->bd_inode->i_blkbits.

submit_bh() has the similar problem.

This patch calculates the block-size using i_blkbits.
So that, for example, __getblk() can get a 4K buffer head while the
block-size of @bdev is 1K.

Signed-off-by: Cong Meng <mcpacino@gmail.com>
---
 fs/buffer.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 3e7dca2..f7f9d33 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1051,10 +1051,7 @@ grow_buffers(struct block_device *bdev, sector_t block, int size)
 	pgoff_t index;
 	int sizebits;
 
-	sizebits = -1;
-	do {
-		sizebits++;
-	} while ((size << sizebits) < PAGE_SIZE);
+	sizebits = PAGE_CACHE_SHIFT - bdev->bd_inode->i_blkbits;
 
 	index = block >> sizebits;
 
@@ -2924,7 +2921,7 @@ int submit_bh(int rw, struct buffer_head * bh)
 	 */
 	bio = bio_alloc(GFP_NOIO, 1);
 
-	bio->bi_sector = bh->b_blocknr * (bh->b_size >> 9);
+	bio->bi_sector = bh->b_blocknr << (bh->b_bdev->bd_inode->i_blkbits - 9);
 	bio->bi_bdev = bh->b_bdev;
 	bio->bi_io_vec[0].bv_page = bh->b_page;
 	bio->bi_io_vec[0].bv_len = bh->b_size;

Re: [PATCH] vfs: calculate block-size correctly

[Andrew Morton]

On Wed, 8 Sep 2010 18:07:10 +0800
Cong Meng <mcpacino@gmail.com> wrote:

> I sent this patch half mount ago, but no response at all. If anything is
> wrong, please tell me. Thanks a lots. This is my first time to send patch
> to linux kernel.
> 
> The invocation of __getblk(bdev, block, size) will cause kernel stall if
> the @size parameter is not equal to the blockr-size of @bdev, which is saved
> in bdev->bd_inode->i_blkbits.
> 
> submit_bh() has the similar problem.
> 
> This patch calculates the block-size using i_blkbits.
> So that, for example, __getblk() can get a 4K buffer head while the
> block-size of @bdev is 1K.
> 

This comes up every couple of years and iirc we always decided that
it's a bug in the calling code somewhere.  How did you hit it?


> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -1051,10 +1051,7 @@ grow_buffers(struct block_device *bdev, sector_t block, int size)
>  	pgoff_t index;
>  	int sizebits;
>  
> -	sizebits = -1;
> -	do {
> -		sizebits++;
> -	} while ((size << sizebits) < PAGE_SIZE);

I'm sure the existing code could use __roundup_pow_of_two() here.

Re: [PATCH] vfs: calculate block-size correctly

[McPacino]

On Tue, Sep 14, 2010 at 7:09 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> On Wed, 8 Sep 2010 18:07:10 +0800
> Cong Meng <mcpacino@gmail.com> wrote:
>
> > I sent this patch half mount ago, but no response at all. If anything is
> > wrong, please tell me. Thanks a lots. This is my first time to send patch
> > to linux kernel.
> >
> > The invocation of __getblk(bdev, block, size) will cause kernel stall if
> > the @size parameter is not equal to the blockr-size of @bdev, which is saved
> > in bdev->bd_inode->i_blkbits.
> >
> > submit_bh() has the similar problem.
> >
> > This patch calculates the block-size using i_blkbits.
> > So that, for example, __getblk() can get a 4K buffer head while the
> > block-size of @bdev is 1K.
> >
>
> This comes up every couple of years and iirc we always decided that
> it's a bug in the calling code somewhere.  How did you hit it?
>

I am working on a new multi-version snapshot target module for device-mapper
which is almost done.  It's on-disk metadata block size is fixed 4K, no matter
what the actual disk block size is. So I call __getblk(bdev, block,
4096) to buffer
the metedata.

This is how I hit it.

grow_dev_page(), the main routine of grow_buffers(), has already supported to
grow page buffer of different size.

If we adapt grow_buffer() this way, imho, It's flexible that make
__getblk() get
buffer head of different size. why not?

>
> > --- a/fs/buffer.c
> > +++ b/fs/buffer.c
> > @@ -1051,10 +1051,7 @@ grow_buffers(struct block_device *bdev, sector_t block, int size)
> >       pgoff_t index;
> >       int sizebits;
> >
> > -     sizebits = -1;
> > -     do {
> > -             sizebits++;
> > -     } while ((size << sizebits) < PAGE_SIZE);
>
> I'm sure the existing code could use __roundup_pow_of_two() here.
>

What does this mean?

(sorry for sneding again. this is plain text.)

Thanks.