Hi,
I re-did the support for mpage_readpages() to map multiple blocks
at a time (basically, get_blocks()). Instead of adding new
get_blocks() and pass it around, I use "bh->b_size" to indicate
how much of disk mapping we are interested in to get_block().
This way no changes existing interfaces and no new routines need.
Filesystem can choose to ignore the passed in "bh->b_size" value
and map one block at a time (ext2, reiser3, etc..)
Currently, I modified JFS & XFS to make use of it and support
multiple blocks and planning to integrate with Mingming's ext3
multiblock work (which is in -mm).
Todo:
1) Integrate with Mingming's multiblock get_block() support for ext3.
2) kill .*_get_blocks() added for DIO ?
Comments ?
Thanks,
Badari
On Tue, Feb 14, 2006 at 09:49:07AM -0800, Badari Pulavarty wrote:
> Hi,
>
> I re-did the support for mpage_readpages() to map multiple blocks
> at a time (basically, get_blocks()). Instead of adding new
> get_blocks() and pass it around, I use "bh->b_size" to indicate
> how much of disk mapping we are interested in to get_block().
>
> This way no changes existing interfaces and no new routines need.
> Filesystem can choose to ignore the passed in "bh->b_size" value
> and map one block at a time (ext2, reiser3, etc..)
Hmm. Given that we only use buffer_heads for page-cache backed I/O
bh->b_size should always be set properly and this would be fine.
If we could completely get rid of ->get_blocks that be a nice cleanup
in all the fs drivers. OTOH I still wonder whether ->get_block should
use a small structure that just contains the information needed with
easy to decipher names..
On Tue, 2006-02-14 at 20:27 +0100, christoph wrote:
> On Tue, Feb 14, 2006 at 09:49:07AM -0800, Badari Pulavarty wrote:
> > Hi,
> >
> > I re-did the support for mpage_readpages() to map multiple blocks
> > at a time (basically, get_blocks()). Instead of adding new
> > get_blocks() and pass it around, I use "bh->b_size" to indicate
> > how much of disk mapping we are interested in to get_block().
> >
> > This way no changes existing interfaces and no new routines need.
> > Filesystem can choose to ignore the passed in "bh->b_size" value
> > and map one block at a time (ext2, reiser3, etc..)
>
> Hmm. Given that we only use buffer_heads for page-cache backed I/O
> bh->b_size should always be set properly and this would be fine.
>
> If we could completely get rid of ->get_blocks that be a nice cleanup
> in all the fs drivers.
Yes. Sir !!
As mentioned in my note, I am already doing that :)
Only problem is, it changes the existing exported interface :(
> OTOH I still wonder whether ->get_block should
> use a small structure that just contains the information needed with
> easy to decipher names..
While ago, I did look at that. There is nothing much to trim in
the bufferhead. Most of this useful for FS, IO or JBD :(
Thanks,
Badari
On Tue, 2006-02-14 at 20:27 +0100, christoph wrote:
> On Tue, Feb 14, 2006 at 09:49:07AM -0800, Badari Pulavarty wrote:
> > Hi,
> >
> > I re-did the support for mpage_readpages() to map multiple blocks
> > at a time (basically, get_blocks()). Instead of adding new
> > get_blocks() and pass it around, I use "bh->b_size" to indicate
> > how much of disk mapping we are interested in to get_block().
> >
> > This way no changes existing interfaces and no new routines need.
> > Filesystem can choose to ignore the passed in "bh->b_size" value
> > and map one block at a time (ext2, reiser3, etc..)
>
> Hmm. Given that we only use buffer_heads for page-cache backed I/O
> bh->b_size should always be set properly and this would be fine.
>
> If we could completely get rid of ->get_blocks that be a nice cleanup
> in all the fs drivers.
Here is the patch to remove ->get_blocks from all fs.
Unfortunately, some filesystems (ext3, reiser3) does extra stuff
in ->getblocks() for DIO. So I can't quite get rid of them :(
But in general, it cleans up a lot.
Thanks,
Badari
Badari Pulavarty wrote:
> + for (i = 0; ; i++) {
> + if (i == nblocks) {
> + *map_valid = 0;
> + break;
> + } else if (page_block == blocks_per_page)
> + break;
> + blocks[page_block] = map_bh->b_blocknr + i;
> + page_block++;
> + block_in_file++;
> + }
Hi Badari, a tiny nit:
for (i = 0; ; i++) {
if (i == nblocks) {
*map_valid = 0;
break;
}
if (page_block == blocks_per_page)
break;
blocks[page_block] = map_bh->b_blocknr + i;
page_block++;
block_in_file++;
}
Regards,
Daniel
On Tue, 2006-02-14 at 15:30 -0800, Daniel Phillips wrote:
> Badari Pulavarty wrote:
> > + for (i = 0; ; i++) {
> > + if (i == nblocks) {
> > + *map_valid = 0;
> > + break;
> > + } else if (page_block == blocks_per_page)
> > + break;
> > + blocks[page_block] = map_bh->b_blocknr + i;
> > + page_block++;
> > + block_in_file++;
> > + }
>
> Hi Badari, a tiny nit:
>
> for (i = 0; ; i++) {
> if (i == nblocks) {
> *map_valid = 0;
> break;
> }
> if (page_block == blocks_per_page)
> break;
> blocks[page_block] = map_bh->b_blocknr + i;
> page_block++;
> block_in_file++;
> }
>
Fixed. Wow !! People really do read others patches. Just kidding :)
Thanks,
Badari