* re: Btrfs: fix locking in btrfs_destroy_delayed_refs
@ 2012-06-18 13:12 Dan Carpenter
2012-06-18 13:20 ` Josef Bacik
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2012-06-18 13:12 UTC (permalink / raw)
To: josef; +Cc: linux-btrfs
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Btrfs: fix locking in btrfs_destroy_delayed_refs
2012-06-18 13:12 Btrfs: fix locking in btrfs_destroy_delayed_refs Dan Carpenter
@ 2012-06-18 13:20 ` Josef Bacik
0 siblings, 0 replies; 2+ messages in thread
From: Josef Bacik @ 2012-06-18 13:20 UTC (permalink / raw)
To: Dan Carpenter; +Cc: josef, linux-btrfs
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-06-18 13:20 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-18 13:12 Btrfs: fix locking in btrfs_destroy_delayed_refs Dan Carpenter
2012-06-18 13:20 ` Josef Bacik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).