* [PATCH 1/1] 2.6.39-rc7+ fs: Fix spinlock recursion in get_active_super()
@ 2011-05-18 16:35 Tim Gardner
2011-05-18 18:15 ` Christoph Hellwig
2011-05-18 23:06 ` Al Viro
0 siblings, 2 replies; 4+ messages in thread
From: Tim Gardner @ 2011-05-18 16:35 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-kernel
>From c7d9161350188c8132210eea5c7f6edff94e6c9c Mon Sep 17 00:00:00 2001
From: Tim Gardner <tim.gardner@canonical.com>
Date: Wed, 18 May 2011 10:30:02 -0600
Subject: [PATCH] fs: Fix spinlock recursion in get_active_super()
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
fs/super.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/super.c b/fs/super.c
index 8a06881..e203e2d 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -503,8 +503,8 @@ struct super_block *get_active_super(struct block_device *bdev)
if (!bdev)
return NULL;
-restart:
spin_lock(&sb_lock);
+restart:
list_for_each_entry(sb, &super_blocks, s_list) {
if (list_empty(&sb->s_instances))
continue;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] 2.6.39-rc7+ fs: Fix spinlock recursion in get_active_super()
2011-05-18 16:35 [PATCH 1/1] 2.6.39-rc7+ fs: Fix spinlock recursion in get_active_super() Tim Gardner
@ 2011-05-18 18:15 ` Christoph Hellwig
2011-05-18 18:51 ` Tim Gardner
2011-05-18 23:06 ` Al Viro
1 sibling, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2011-05-18 18:15 UTC (permalink / raw)
To: Tim Gardner; +Cc: viro, linux-fsdevel, linux-kernel
How can you reproduce that recursion? Given that grab_super drops
sb_lock I can't see any way to hit it.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] 2.6.39-rc7+ fs: Fix spinlock recursion in get_active_super()
2011-05-18 18:15 ` Christoph Hellwig
@ 2011-05-18 18:51 ` Tim Gardner
0 siblings, 0 replies; 4+ messages in thread
From: Tim Gardner @ 2011-05-18 18:51 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: viro, linux-fsdevel, linux-kernel
On 05/18/2011 12:15 PM, Christoph Hellwig wrote:
> How can you reproduce that recursion? Given that grab_super drops
> sb_lock I can't see any way to hit it.
>
Dang, you're right. Consider it NAKd
rtg
--
Tim Gardner tim.gardner@canonical.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] 2.6.39-rc7+ fs: Fix spinlock recursion in get_active_super()
2011-05-18 16:35 [PATCH 1/1] 2.6.39-rc7+ fs: Fix spinlock recursion in get_active_super() Tim Gardner
2011-05-18 18:15 ` Christoph Hellwig
@ 2011-05-18 23:06 ` Al Viro
1 sibling, 0 replies; 4+ messages in thread
From: Al Viro @ 2011-05-18 23:06 UTC (permalink / raw)
To: Tim Gardner; +Cc: linux-fsdevel, linux-kernel
On Wed, May 18, 2011 at 10:35:00AM -0600, Tim Gardner wrote:
> >From c7d9161350188c8132210eea5c7f6edff94e6c9c Mon Sep 17 00:00:00 2001
> From: Tim Gardner <tim.gardner@canonical.com>
> Date: Wed, 18 May 2011 10:30:02 -0600
> Subject: [PATCH] fs: Fix spinlock recursion in get_active_super()
>
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
> fs/super.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/super.c b/fs/super.c
> index 8a06881..e203e2d 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -503,8 +503,8 @@ struct super_block *get_active_super(struct block_device *bdev)
> if (!bdev)
> return NULL;
>
> -restart:
> spin_lock(&sb_lock);
> +restart:
> list_for_each_entry(sb, &super_blocks, s_list) {
> if (list_empty(&sb->s_instances))
> continue;
WTF? Have you even tried that? The *only* place that contains goto restart
is a few line below and it's
if (grab_super(sb)) /* drops sb_lock */
return sb;
else
goto restart;
See that comment in there? Now let's see if it's true:
static int grab_super(struct super_block *s) __releases(sb_lock)
{
if (atomic_inc_not_zero(&s->s_active)) {
spin_unlock(&sb_lock);
return 1;
}
/* it's going away */
s->s_count++;
spin_unlock(&sb_lock);
/* wait for it to die */
down_write(&s->s_umount);
up_write(&s->s_umount);
put_super(s);
return 0;
}
Note spin_unlock on both paths. Morever, note blocking operations on the
path that returns 0. If we had somehow managed to get through that without
dropping sb_locked we'd be FUBAR for obvious reasons.
IOW, if your testing had *ever* hit that goto, you'd get instant trouble.
On the exit from get_active_super() you'd hit spin_unlock(&sb_lock), with
rather nasty consequences the next time somebody would try to get it...
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-05-18 23:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-18 16:35 [PATCH 1/1] 2.6.39-rc7+ fs: Fix spinlock recursion in get_active_super() Tim Gardner
2011-05-18 18:15 ` Christoph Hellwig
2011-05-18 18:51 ` Tim Gardner
2011-05-18 23:06 ` Al Viro
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.