Hi,
On Mon, Apr 02, 2012 at 12:08:54PM +0300, Dan Carpenter wrote:
> The patch 49b25e054090: "btrfs: enhance transaction abort
> infrastructure" from Mar 1, 2012, leads to the following warning:
> fs/btrfs/transaction.c:137 join_transaction()
> warn: inconsistent returns spin_lock:&root->fs_info->trans_lock:
> locked (77) unlocked (64,70,82,88,137)
>
> fs/btrfs/transaction.c
> 74 cur_trans = root->fs_info->running_transaction;
> 75 if (cur_trans) {
> 76 if (cur_trans->aborted)
> 77 return cur_trans->aborted;
> ^^^^^^^^^^^^^^^^^^^^^^^^^^
> Should we do a spin_unlock(&root->fs_info->trans_lock); here before
> returning?
Yes.
>
> 78 atomic_inc(&cur_trans->use_count);
> 79 atomic_inc(&cur_trans->num_writers);
> 80 cur_trans->num_joined++;
> 81 spin_unlock(&root->fs_info->trans_lock);
> 82 return 0;
> 83 }
> 84 spin_unlock(&root->fs_info->trans_lock);
>
> Also later on:
>
> 1399 ret = commit_fs_roots(trans, root);
> 1400 if (ret) {
> 1401 mutex_unlock(&root->fs_info->tree_log_mutex);
> 1402 goto cleanup_transaction;
> ^^^^^^^^^^^^^^^^^^^^^^^^^
> 1403 }
> 1404
> 1405 /* commit_fs_roots gets rid of all the tree log roots, it is now
> 1406 * safe to free the root of tree log roots
> 1407 */
> 1408 btrfs_free_log_root_tree(trans, root->fs_info);
> 1409
> 1410 ret = commit_cowonly_roots(trans, root);
> 1411 if (ret) {
> 1412 mutex_unlock(&root->fs_info->tree_log_mutex);
> 1413 goto cleanup_transaction;
> ^^^^^^^^^^^^^^^^^^^^^^^^^
>
> Should we do an mutex_unlock(&root->fs_info->reloc_mutex); before these
> two gotos?
Yes.
Thanks for cathing it. Are you going to send a patch or are you fine
with a Reported-by: ?
david
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
