On Mon, Feb 18, 2008 at 04:14:34PM -0800, Mingming Cao wrote:
> On Sat, 2008-02-16 at 08:53 +0530, Aneesh Kumar K.V wrote:
> How about the following patch?
> Regards,
> Mingming
> ext4: ext4_get_blocks_wrap fix for writing to preallocated
> From: Mingming Cao <[EMAIL PROTECTED]>
> This patch fixed a issue with wrting to a preallocated blocks.
> A write hit a BUG_ON() in fs/buffer.c saying the buffer is not mapped.
> On the write path, ext4_get_block_wrap() is called with create=1, but it
> will pass create=0 down to the underlying ext4ext_get_blocks()
> to do a look up first.  In the preallocation case, ext4_ext_get_blocks()
> with create = 0, will return number of blocks pre-allocated and buffer
> head unmapped. ext4_get_blocks_wrap() thinks it succeeds too early, without
> checking if it needs again call ext4_ext_get_blocks with create = 1
> which would do proper handling for writing to preallocated blocks:
> split the extent to initialized and uninitialized one and
> returns the mapped buffer head.
> Treating preallocated blocks as holes equally(i.e. ignoring the number of 
> blocks
> pre-allocated and returns 0) when get_blocks() is called with create = 0 is 
> not enough.
> ext4_ext_get_blocks() needs to differentiate these two cases for delayed 
> allocation
> purpose, as for holes it need to do reservation and prepare for later
> delayed allocation, but for pre-allocated blocks it needs skip that work.
> It would makes things more clear if we have clear definition of what
> get_blocks() return value means.
> Similar to ext4_get_blocks_handle(),  the following 
> * return > 0, # of blocks already allocated
> *          if these are pre-allocated blocks and create = 0
> *             buffer head is unmapped
> *          otherwise blocks are mapped.
> *
> * return = 0, if plain look up failed (blocks have not been allocated)
> *          buffer head is unmapped
> *
> * return < 0, error case.
> The for the write path, at ext4_ext_get_blocks_wrap(), it could check the
> buffer_mapped() status for preallocated extent before quit too early.
> Signed-off-by: Mingming Cao <[EMAIL PROTECTED]>

Reviewed-by: Aneesh Kumar K.V <[EMAIL PROTECTED]>

I guess we also need to make sure the buffer head have the mapped bit
set. Something like the patch below.

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index bc7081f..69ccda9 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2294,6 +2294,7 @@ int ext4_ext_get_blocks(handle_t *handle, struct inode 
        struct ext4_allocation_request ar;
        __clear_bit(BH_New, &bh_result->b_state);
+       __clear_bit(BH_Mapped, &bh_result->b_state);
        ext_debug("blocks %u/%lu requested for inode %u\n",
                        iblock, max_blocks, inode->i_ino);
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to