On Tue, Jun 03, 2008 at 10:23:56PM -0400, Theodore Tso wrote: > On Tue, May 20, 2008 at 02:04:22AM +0530, Aneesh Kumar K.V wrote: > > When mouting ext4 with -o noextents, request for > > file data blocks from inode prealloc space. > > Aneesh, can you expand on this patch set? Why is this important? > What was it doing beforehand? Is this to replace the use of the block > reservations code that had been introduced by Mingming in ext3? Or is > that a long-term goal? > > Also, it would be nice to add some comments at the beginning of the > changed functions, explaining what the functions do, what they are > intended for, what they assumptions they make (i.e., do they assume > that certain locks are taken), any side effects they make (i.e., this > function calls get_bh or put_bh to change the refcount on a passed-in > buffer head). > > I assume there was a good reason that you renamed the function > ext4_new_block() to ext4_fsblk_t ext4_new_meta_block(), but why? Some > comments would really be useful. > > I asked Mingming what this patch did when I was reviewing it this > afternoon, since we were both in New Hampshire at the btrfs > conference. I couldn't parse the english for the comments, and after > looking at the patch, it wasn't at all obvious what the patch was > trying to accomplish. Even though Mingming had reviewed it maybe two > weeks ago, she couldn't figure it out completely after looking over > the patch. Consider how a someone who isn't intimately familiar with > the code would be able to figure out either (a) the code, or (b) > looking over the changeset. Good code has to be long-term > maintainable, and some comments would really help in this department. > > Since neither of us couldn't figure it out what the motivation of this > patch, we've decided to move it to the unstable portion of the patch > since without some better comments, it's probably not a good idea to > push it to Linus in this form. > > - Ted When we mount the filesystem with -o noextents currently the block allocation requests are not using the preallocation feature of mballoc. mballoc consider the allocation request that doesn't have ar.flags = EXT4_MB_HINT_DATA set as meta-data allocation and force them to get the blocks via regular allocator ext4_mb_regular_allocator. In order to make sure -o noextents (or ext3 format) block allocation use the preallocation feature we need to set the EXT4_MB_HINT_DATA while requesting for the blocks. mballoc also needs the logical block number of the requested block so that it call nicely place it in the inode prealloc space. This means that we need to differentiate between data allocation request and meta-data allocation request. The meta-data allocation is done without setting EXT4_MB_HINT_DATA flag and they would use the regular allocator where as the data blocks are requested with EXT4_MB_HINT_DATA set and along with logical block number so that they can allocate the blocks nicely from the inode prealloc space. Now with -o noextents we request for blocks via ext4_alloc_blocks. Earlier we request for both the data and meta-block together. Now we can't do that since the data-blocks are allocated based on the logical block number of the request. So the changes was to split it such that we first request for meta-data blocks and then request for data-blocks with the logical block number. -aneesh -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
