Hello Josef Bacik,
The patch b939d1ab76b4: "Btrfs: fix locking in
btrfs_destroy_delayed_refs" from May 31, 2012, leads to the following
warning: Btrfs: fix locking in btrfs_destroy_delayed_refs
fs/btrfs/disk-io.c
3412 while ((node = rb_first(&delayed_refs->root)) != NULL) {
3413 ref = rb_entry(node, struct btrfs_delayed_ref_node,
rb_node);
3414
3415 atomic_set(&ref->refs, 1);
3416 if (btrfs_delayed_ref_is_head(ref)) {
3417 struct btrfs_delayed_ref_head *head;
3418
3419 head = btrfs_delayed_node_to_head(ref);
3420 if (!mutex_trylock(&head->mutex)) {
3421 atomic_inc(&ref->refs);
3422 spin_unlock(&delayed_refs->lock);
3423
3424 /* Need to wait for the delayed ref to
run */
3425 mutex_lock(&head->mutex);
3426 mutex_unlock(&head->mutex);
3427 btrfs_put_delayed_ref(ref);
3428
3429 continue;
^^^^^^^^^
We''re not holding the &delayed_refs->lock here.
3430 }
3431
3432 kfree(head->extent_op);
3433 delayed_refs->num_heads--;
3434 if (list_empty(&head->cluster))
3435 delayed_refs->num_heads_ready--;
3436 list_del_init(&head->cluster);
3437 }
3438 ref->in_tree = 0;
3439 rb_erase(&ref->rb_node,
&delayed_refs->root);
3440 delayed_refs->num_entries--;
3441
3442 spin_unlock(&delayed_refs->lock);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
So this is a double unlock.
3443 btrfs_put_delayed_ref(ref);
3444
3445 cond_resched();
3446 spin_lock(&delayed_refs->lock);
3447 }
3448
3449 spin_unlock(&delayed_refs->lock);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Or if we exit, then this is a double unlock.
There is some complicated locking going on in that function so I don''t
pretend to understand it. Sorry, if I''ve misread something.
regards,
dan carpenter
--
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
On Mon, Jun 18, 2012 at 04:12:39PM +0300, Dan Carpenter wrote:> Hello Josef Bacik, > > The patch b939d1ab76b4: "Btrfs: fix locking in > btrfs_destroy_delayed_refs" from May 31, 2012, leads to the following > warning: Btrfs: fix locking in btrfs_destroy_delayed_refs > > fs/btrfs/disk-io.c > 3412 while ((node = rb_first(&delayed_refs->root)) != NULL) { > 3413 ref = rb_entry(node, struct btrfs_delayed_ref_node, rb_node); > 3414 > 3415 atomic_set(&ref->refs, 1); > 3416 if (btrfs_delayed_ref_is_head(ref)) { > 3417 struct btrfs_delayed_ref_head *head; > 3418 > 3419 head = btrfs_delayed_node_to_head(ref); > 3420 if (!mutex_trylock(&head->mutex)) { > 3421 atomic_inc(&ref->refs); > 3422 spin_unlock(&delayed_refs->lock); > 3423 > 3424 /* Need to wait for the delayed ref to run */ > 3425 mutex_lock(&head->mutex); > 3426 mutex_unlock(&head->mutex); > 3427 btrfs_put_delayed_ref(ref); > 3428 > 3429 continue; > ^^^^^^^^^ > We''re not holding the &delayed_refs->lock here. > > 3430 } > 3431 > 3432 kfree(head->extent_op); > 3433 delayed_refs->num_heads--; > 3434 if (list_empty(&head->cluster)) > 3435 delayed_refs->num_heads_ready--; > 3436 list_del_init(&head->cluster); > 3437 } > 3438 ref->in_tree = 0; > 3439 rb_erase(&ref->rb_node, &delayed_refs->root); > 3440 delayed_refs->num_entries--; > 3441 > 3442 spin_unlock(&delayed_refs->lock); > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > So this is a double unlock. > > 3443 btrfs_put_delayed_ref(ref); > 3444 > 3445 cond_resched(); > 3446 spin_lock(&delayed_refs->lock); > 3447 } > 3448 > 3449 spin_unlock(&delayed_refs->lock); > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > Or if we exit, then this is a double unlock. > > There is some complicated locking going on in that function so I don''t > pretend to understand it. Sorry, if I''ve misread something. >Ooops, nope you are right, I''ll fix it. Thanks, Josef -- 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