Jan Schmidt
2013-Jan-28 06:26 UTC
[PATCH] Btrfs: fix EDQUOT handling in btrfs_delalloc_reserve_metadata
When btrfs_qgroup_reserve returned a failure, we were missing a counter operation for BTRFS_I(inode)->outstanding_extents++, leading to warning messages about outstanding extents and space_info->bytes_may_use != 0. Additionally, the error handling code didn''t take into account that we dropped the inode lock which might require more cleanup. Luckily, all the cleanup code we need is already there and can be shared with reserve_metadata_bytes, which is exactly what this patch does. Reported-by: Lev Vainblat <lev@zadarastorage.com> Signed-off-by: Jan Schmidt <list.btrfs@jan-o-sch.net> --- fs/btrfs/extent-tree.c | 22 ++++++++++------------ 1 files changed, 10 insertions(+), 12 deletions(-) diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 85b8454..61da9d0 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -4534,7 +4534,7 @@ int btrfs_delalloc_reserve_metadata(struct inode *inode, u64 num_bytes) unsigned nr_extents = 0; int extra_reserve = 0; enum btrfs_reserve_flush_enum flush = BTRFS_RESERVE_FLUSH_ALL; - int ret; + int ret = 0; bool delalloc_lock = true; /* If we are a free space inode we need to not flush since we will be in @@ -4579,20 +4579,18 @@ int btrfs_delalloc_reserve_metadata(struct inode *inode, u64 num_bytes) csum_bytes = BTRFS_I(inode)->csum_bytes; spin_unlock(&BTRFS_I(inode)->lock); - if (root->fs_info->quota_enabled) { + if (root->fs_info->quota_enabled) ret = btrfs_qgroup_reserve(root, num_bytes + nr_extents * root->leafsize); - if (ret) { - spin_lock(&BTRFS_I(inode)->lock); - calc_csum_metadata_size(inode, num_bytes, 0); - spin_unlock(&BTRFS_I(inode)->lock); - if (delalloc_lock) - mutex_unlock(&BTRFS_I(inode)->delalloc_mutex); - return ret; - } - } - ret = reserve_metadata_bytes(root, block_rsv, to_reserve, flush); + /* + * ret != 0 here means the qgroup reservation failed, we go straight to + * the shared error handling then. + */ + if (ret == 0) + ret = reserve_metadata_bytes(root, block_rsv, + to_reserve, flush); + if (ret) { u64 to_free = 0; unsigned dropped; -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Liu Bo
2013-Jan-28 08:55 UTC
Re: [PATCH] Btrfs: fix EDQUOT handling in btrfs_delalloc_reserve_metadata
On Mon, Jan 28, 2013 at 07:26:00AM +0100, Jan Schmidt wrote:> When btrfs_qgroup_reserve returned a failure, we were missing a counter > operation for BTRFS_I(inode)->outstanding_extents++, leading to warning > messages about outstanding extents and space_info->bytes_may_use != 0. > Additionally, the error handling code didn''t take into account that we > dropped the inode lock which might require more cleanup. > > Luckily, all the cleanup code we need is already there and can be shared > with reserve_metadata_bytes, which is exactly what this patch does.Reviewed-by: Liu Bo <bo.li.liu@oracle.com>> > Reported-by: Lev Vainblat <lev@zadarastorage.com> > Signed-off-by: Jan Schmidt <list.btrfs@jan-o-sch.net> > --- > fs/btrfs/extent-tree.c | 22 ++++++++++------------ > 1 files changed, 10 insertions(+), 12 deletions(-) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 85b8454..61da9d0 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -4534,7 +4534,7 @@ int btrfs_delalloc_reserve_metadata(struct inode *inode, u64 num_bytes) > unsigned nr_extents = 0; > int extra_reserve = 0; > enum btrfs_reserve_flush_enum flush = BTRFS_RESERVE_FLUSH_ALL; > - int ret; > + int ret = 0; > bool delalloc_lock = true; > > /* If we are a free space inode we need to not flush since we will be in > @@ -4579,20 +4579,18 @@ int btrfs_delalloc_reserve_metadata(struct inode *inode, u64 num_bytes) > csum_bytes = BTRFS_I(inode)->csum_bytes; > spin_unlock(&BTRFS_I(inode)->lock); > > - if (root->fs_info->quota_enabled) { > + if (root->fs_info->quota_enabled) > ret = btrfs_qgroup_reserve(root, num_bytes + > nr_extents * root->leafsize); > - if (ret) { > - spin_lock(&BTRFS_I(inode)->lock); > - calc_csum_metadata_size(inode, num_bytes, 0); > - spin_unlock(&BTRFS_I(inode)->lock); > - if (delalloc_lock) > - mutex_unlock(&BTRFS_I(inode)->delalloc_mutex); > - return ret; > - } > - } > > - ret = reserve_metadata_bytes(root, block_rsv, to_reserve, flush); > + /* > + * ret != 0 here means the qgroup reservation failed, we go straight to > + * the shared error handling then. > + */ > + if (ret == 0) > + ret = reserve_metadata_bytes(root, block_rsv, > + to_reserve, flush); > + > if (ret) { > u64 to_free = 0; > unsigned dropped; > -- > 1.7.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html-- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html