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 <cmm@xxxxxxxxxx>
>
> 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 <cmm@xxxxxxxxxx>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxx>
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 *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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
