Re: [PATCH] ext4: fix sleep inside spinlock issue aka #14739 V2

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

On Thu, Dec 10, 2009 at 08:22:16PM +0300, Dmitry Monakhov wrote:
> 
> drop i_block_reservation_lock before vfs_dq_reserve_block().
> this patch fix http://bugzilla.kernel.org/show_bug.cgi?id=14739
> 
> changes from previous version:
>  - simplify the patch according to Jan's comments
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
>  fs/ext4/inode.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 942e183..2327f7a 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1851,6 +1851,7 @@ repeat:
> 
>  	md_needed = mdblocks - EXT4_I(inode)->i_reserved_meta_blocks;
>  	total = md_needed + nrblocks;
> +	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
> 
>  	/*
>  	 * Make quota reservation here to prevent quota overflow
> @@ -1858,12 +1859,10 @@ repeat:
>  	 * time.
>  	 */
>  	if (vfs_dq_reserve_block(inode, total)) {
> -		spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
>  		return -EDQUOT;
>  	}
> 
>  	if (ext4_claim_free_blocks(sbi, total)) {
> -		spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
>  		vfs_dq_release_reservation_block(inode, total);
>  		if (ext4_should_retry_alloc(inode->i_sb, &retries)) {
>  			yield();
> @@ -1871,10 +1870,11 @@ repeat:
>  		}
>  		return -ENOSPC;
>  	}
> +	spin_lock(&EXT4_I(inode)->i_block_reservation_lock);
>  	EXT4_I(inode)->i_reserved_data_blocks += nrblocks;
>  	EXT4_I(inode)->i_reserved_meta_blocks = mdblocks;
> -
>  	spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
> +
>  	return 0;       /* success */


NACK
I guess we would end up setting i_reserved_meta_blocks wrongly because
mdblocks could be based on the old value of i_reserved_meta_blocks
because we are dropping i_block_reservation_lock lock.

-aneesh