On Wed, 2009-12-09 at 05:11 +0300, Dmitry Monakhov wrote: > - ext4_getattr() not longer required > - ext4_get_reserved_space() not longer required > - drop i_block_reservation_lock before vfs_dq_reserve_block(). > this fix http://bugzilla.kernel.org/show_bug.cgi?id=14739diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 26d3cf8..6198751 100644 > How about split this patch to just drop i_block_reservation_lock before vfs_dq_reserve_block()? This deadlock could be fixed right away without depending on the rest of the VFS changes you proposed. Mingming > Signed-off-by: Dmitry Monakhov <dmonakhov@xxxxxxxxxx> > --- > fs/ext4/ext4.h | 2 - > fs/ext4/file.c | 1 - > fs/ext4/inode.c | 67 +++++++++++++----------------------------------------- > fs/ext4/super.c | 1 - > 4 files changed, 16 insertions(+), 55 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 26d3cf8..6198751 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -1407,8 +1407,6 @@ int ext4_get_block(struct inode *inode, sector_t iblock, > extern struct inode *ext4_iget(struct super_block *, unsigned long); > extern int ext4_write_inode(struct inode *, int); > extern int ext4_setattr(struct dentry *, struct iattr *); > -extern int ext4_getattr(struct vfsmount *mnt, struct dentry *dentry, > - struct kstat *stat); > extern void ext4_delete_inode(struct inode *); > extern int ext4_sync_inode(handle_t *, struct inode *); > extern void ext4_dirty_inode(struct inode *); > diff --git a/fs/ext4/file.c b/fs/ext4/file.c > index 9630583..36a75e7 100644 > --- a/fs/ext4/file.c > +++ b/fs/ext4/file.c > @@ -151,7 +151,6 @@ const struct file_operations ext4_file_operations = { > const struct inode_operations ext4_file_inode_operations = { > .truncate = ext4_truncate, > .setattr = ext4_setattr, > - .getattr = ext4_getattr, > #ifdef CONFIG_EXT4_FS_XATTR > .setxattr = generic_setxattr, > .getxattr = generic_getxattr, > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index cc4737e..9a27505 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -1048,17 +1048,6 @@ out: > return err; > } > > -qsize_t ext4_get_reserved_space(struct inode *inode) > -{ > - unsigned long long total; > - > - spin_lock(&EXT4_I(inode)->i_block_reservation_lock); > - total = EXT4_I(inode)->i_reserved_data_blocks + > - EXT4_I(inode)->i_reserved_meta_blocks; > - spin_unlock(&EXT4_I(inode)->i_block_reservation_lock); > - > - return (total << inode->i_blkbits); > -} > /* > * Calculate the number of metadata blocks need to reserve > * to allocate @blocks for non extent file based file > @@ -1852,19 +1841,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 +1850,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) > @@ -5511,33 +5503,6 @@ err_out: > return error; > } > > -int ext4_getattr(struct vfsmount *mnt, struct dentry *dentry, > - struct kstat *stat) > -{ > - struct inode *inode; > - unsigned long delalloc_blocks; > - > - inode = dentry->d_inode; > - generic_fillattr(inode, stat); > - > - /* > - * We can't update i_blocks if the block allocation is delayed > - * otherwise in the case of system crash before the real block > - * allocation is done, we will have i_blocks inconsistent with > - * on-disk file blocks. > - * We always keep i_blocks updated together with real > - * allocation. But to not confuse with user, stat > - * will return the blocks that include the delayed allocation > - * blocks for this file. > - */ > - spin_lock(&EXT4_I(inode)->i_block_reservation_lock); > - delalloc_blocks = EXT4_I(inode)->i_reserved_data_blocks; > - spin_unlock(&EXT4_I(inode)->i_block_reservation_lock); > - > - stat->blocks += (delalloc_blocks << inode->i_sb->s_blocksize_bits)>>9; > - return 0; > -} > - > static int ext4_indirect_trans_blocks(struct inode *inode, int nrblocks, > int chunk) > { > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 4c87d97..d87c7f7 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -1007,7 +1007,6 @@ static const struct dquot_operations ext4_quota_operations = { > .reserve_space = dquot_reserve_space, > .claim_space = dquot_claim_space, > .release_rsv = dquot_release_reserved_space, > - .get_reserved_space = ext4_get_reserved_space, > .alloc_inode = dquot_alloc_inode, > .free_space = dquot_free_space, > .free_inode = dquot_free_inode, -- 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
