On Fri, Dec 12, 2008 at 09:56:09AM -0500, Theodore Tso wrote:

On Tue, Dec 09, 2008 at 04:11:22PM +0530, Aneesh Kumar K.V wrote:

quoted

The problem is due to remove-do_blk_alloc patch.

The patch below should fix the crash. 

-		EXT4_I(inode)->i_allocated_meta_blocks += *count;
+		EXT4_I(inode)->i_allocated_meta_blocks += ar.len;

Good catch, thanks.  I'll add it to the patch queue.

quoted

I have one question regarding the patch. What about blocks allocated for
directories for the  ext3 format.  With extent format we are not
setting EXT4_MB_HINT_DATA for non regular files. So i guess we also
need the below patch .

One of the good things about getting rid of too many layers of
abstractions is that it makes bugs like this easier to spot.  We've
been sending allocating directory and symlinks using EXT4_MB_HINT_DATA
if extents haven't been enabled, and no one noticed before we
simplified out things....

We had always sent the directory allocation request with
EXT4_MB_HINT_DATA not set.  With Linus kernel I have this

671 static ext4_fsblk_t do_blk_alloc(handle_t *handle, struct inode *inode,
.....
....
686         if (S_ISREG(inode->i_mode) && !(flags & EXT4_META_BLOCK))
687                 /* enable in-core preallocation for data block allocation */
688                 ar.flags = EXT4_MB_HINT_DATA;
689         else
690                 /* disable in-core preallocation for non-regular files */
691                 ar.flags = 0;


That means if the request for block allocation is not on regular files
set ar.flags = 0; For regular files if the request is for meta-data
blocks set ar.glags = 0.

Actually, I wonder if maybe we should set EXT4_MB_HINT_DATA for
directories as well.  Making directories contiguous does speed up
certain workloads, and it does speed up fsck.  It may be though that
the mballoc algorithms should be tuned specifically for directories,
and what we should do is to define a new flag, EXT4_MB_HINT_DIRECTORY,
and pass it in for that case. 

Some experimentation is clearly called for, here....

True. But with the changes to do do_blk_alloc I guess we need to make
sure we request directories with EXT4_MB_HINT_DATA not set.

-aneesh