| Message ID | 1430412425-12876-1-git-send-email-fdmanana@suse.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, Apr 30, 2015 at 05:47:05PM +0100, Filipe Manana wrote: > If the call to btrfs_truncate_inode_items() failed and we don't have a block > group, we were unlocking the cache_write_mutex without having locked it (we > do it only if we have a block group). > > Fixes: 1bbc621ef284 ("Btrfs: allow block group cache writeout > outside critical section in commit") > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Patch is ok, I have one comment to the transaction abort calls. The pattern is to put them right after the error is detected so we can trace it back to the sources based on the reports. > @@ -231,6 +231,7 @@ int btrfs_truncate_free_space_cache(struct btrfs_root *root, > { > int ret = 0; > struct btrfs_path *path = btrfs_alloc_path(); > + bool locked = false; > > if (!path) { > ret = -ENOMEM; the context: goto fail; > @@ -269,18 +271,14 @@ int btrfs_truncate_free_space_cache(struct btrfs_root *root, > */ > ret = btrfs_truncate_inode_items(trans, root, inode, > 0, BTRFS_EXTENT_DATA_KEY); > - if (ret) { > - mutex_unlock(&trans->transaction->cache_write_mutex); > - btrfs_abort_transaction(trans, root, ret); > - return ret; > - } > + if (ret) > + goto fail; > > ret = btrfs_update_inode(trans, root, inode); > > - if (block_group) > - mutex_unlock(&trans->transaction->cache_write_mutex); > - > fail: > + if (locked) > + mutex_unlock(&trans->transaction->cache_write_mutex); > if (ret) > btrfs_abort_transaction(trans, root, ret); Now two code paths lead to one abort. We can get ENOMEM from both, however this kind of error is not really a bug we want to analyze deeper so I'm fine with it. Reviewed-by: David Sterba <dsterba@suse.cz> -- 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
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c index c1d1e6d..f1ddccf 100644 --- a/fs/btrfs/free-space-cache.c +++ b/fs/btrfs/free-space-cache.c @@ -231,6 +231,7 @@ int btrfs_truncate_free_space_cache(struct btrfs_root *root, { int ret = 0; struct btrfs_path *path = btrfs_alloc_path(); + bool locked = false; if (!path) { ret = -ENOMEM; @@ -238,6 +239,7 @@ int btrfs_truncate_free_space_cache(struct btrfs_root *root, } if (block_group) { + locked = true; mutex_lock(&trans->transaction->cache_write_mutex); if (!list_empty(&block_group->io_list)) { list_del_init(&block_group->io_list); @@ -269,18 +271,14 @@ int btrfs_truncate_free_space_cache(struct btrfs_root *root, */ ret = btrfs_truncate_inode_items(trans, root, inode, 0, BTRFS_EXTENT_DATA_KEY); - if (ret) { - mutex_unlock(&trans->transaction->cache_write_mutex); - btrfs_abort_transaction(trans, root, ret); - return ret; - } + if (ret) + goto fail; ret = btrfs_update_inode(trans, root, inode); - if (block_group) - mutex_unlock(&trans->transaction->cache_write_mutex); - fail: + if (locked) + mutex_unlock(&trans->transaction->cache_write_mutex); if (ret) btrfs_abort_transaction(trans, root, ret);
If the call to btrfs_truncate_inode_items() failed and we don't have a block group, we were unlocking the cache_write_mutex without having locked it (we do it only if we have a block group). Fixes: 1bbc621ef284 ("Btrfs: allow block group cache writeout outside critical section in commit") Signed-off-by: Filipe Manana <fdmanana@suse.com> --- fs/btrfs/free-space-cache.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)