> drop i_block_reservation_lock before vfs_dq_reserve_block(). > this patch fix http://bugzilla.kernel.org/show_bug.cgi?id=14739 > > Signed-off-by: Dmitry Monakhov <dmonakhov@xxxxxxxxxx> Sorry if someone already refused this (I didn't follow the previous discussion too closely) but: Looking at the code I see no reason why ext4_claim_free_blocks needs i_block_reservation_lock. In fact mballoc calls this function without the lock. So could not we just compute 'total' under the lock, release it, reserve quota and then claim free blocks? You'd get rid of undoing the block reservation and obtain quota and blocks in the usual order... Honza > --- > fs/ext4/inode.c | 29 ++++++++++++++++------------- > 1 files changed, 16 insertions(+), 13 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 942e183..f693768 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -1852,19 +1852,8 @@ repeat: > md_needed = mdblocks - EXT4_I(inode)->i_reserved_meta_blocks; > total = md_needed + nrblocks; > > - /* > - * Make quota reservation here to prevent quota overflow > - * later. Real quota accounting is done at pages writeout > - * 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(); > goto repeat; > @@ -1872,10 +1861,24 @@ repeat: > return -ENOSPC; > } > EXT4_I(inode)->i_reserved_data_blocks += nrblocks; > - EXT4_I(inode)->i_reserved_meta_blocks = mdblocks; > + EXT4_I(inode)->i_reserved_meta_blocks += md_needed; > + spin_unlock(&EXT4_I(inode)->i_block_reservation_lock); > + > + /* > + * Make quota reservation here to prevent quota overflow > + * later. Real quota accounting is done at pages writeout > + * time. > + */ > + if (!vfs_dq_reserve_block(inode, total)) > + return 0; /* success */ > > + /* Quota reservation has failed, revert inode's reservation */ > + percpu_counter_sub(&sbi->s_dirtyblocks_counter, total); > + spin_lock(&EXT4_I(inode)->i_block_reservation_lock); > + EXT4_I(inode)->i_reserved_data_blocks -= nrblocks; > + EXT4_I(inode)->i_reserved_meta_blocks -= md_needed; > spin_unlock(&EXT4_I(inode)->i_block_reservation_lock); > - return 0; /* success */ > + return -EDQUOT; > } > > static void ext4_da_release_space(struct inode *inode, int to_free) > -- > 1.6.0.4 > > -- > 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 -- Jan Kara <jack@xxxxxxx> SuSE CR Labs -- 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
