On Tue, 2008-02-19 at 08:49 +0530, Aneesh Kumar K.V wrote: > 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. > Good point. I modified the patch with clear_buffer_mapped() added at the begining of the wrapper function.
Mingming > 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html