* 2.6.31rc5 RAID10 lockdep report
@ 2009-08-04 14:14 Dave Jones
2009-08-04 21:13 ` 2.6.31rc5 RAID10 lockdep report - sysfs_nofity_dirent locking issue NeilBrown
0 siblings, 1 reply; 7+ messages in thread
From: Dave Jones @ 2009-08-04 14:14 UTC (permalink / raw)
To: Linux Kernel; +Cc: neilb
Report from a user we received today ..
(https://bugzilla.redhat.com/show_bug.cgi?id=515471)
Dave
=================================
[ INFO: inconsistent lock state ]
2.6.31-0.118.rc5.fc12.x86_64 #1
---------------------------------
inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
md126_resync/475 [HC0[0]:SC1[1]:HE1:SE0] takes:
(sysfs_open_dirent_lock){+.?...}, at: [<ffffffff811a7d08>] sysfs_notify_dirent+0x2c/0x75
{SOFTIRQ-ON-W} state was registered at:
[<ffffffff81097468>] __lock_acquire+0x2e9/0xc0e
[<ffffffff81097e7b>] lock_acquire+0xee/0x12e
[<ffffffff814fc99f>] _spin_lock+0x45/0x8e
[<ffffffff811a70e0>] sysfs_open_file+0x18f/0x295
[<ffffffff8113fba1>] __dentry_open+0x197/0x316
[<ffffffff8113fe20>] nameidata_to_filp+0x51/0x76
[<ffffffff8114e900>] do_filp_open+0x516/0x9d8
[<ffffffff8113f889>] do_sys_open+0x71/0x131
[<ffffffff8113f9b6>] sys_open+0x33/0x49
[<ffffffff81012f42>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
irq event stamp: 12243532
hardirqs last enabled at (12243532): [<ffffffff814fc69b>] _spin_unlock_irq+0x3f/0x61
hardirqs last disabled at (12243531): [<ffffffff814fcaa9>] _spin_lock_irq+0x2e/0x9a
softirqs last enabled at (12243378): [<ffffffff8106c9ad>] __do_softirq+0x1c4/0x1f0
softirqs last disabled at (12243471): [<ffffffff8101422c>] call_softirq+0x1c/0x30
other info that might help us debug this:
1 lock held by md126_resync/475:
#0: (&new->safemode_timer){+.-...}, at: [<ffffffff8107233a>] run_timer_softirq+0x198/0x2a3
stack backtrace:
Pid: 475, comm: md126_resync Not tainted 2.6.31-0.118.rc5.fc12.x86_64 #1
Call Trace:
<IRQ> [<ffffffff81095f60>] valid_state+0x187/0x1ae
[<ffffffff810969f1>] ? check_usage_forwards+0x0/0x79
[<ffffffff810960b0>] mark_lock+0x129/0x253
[<ffffffff810973f4>] __lock_acquire+0x275/0xc0e
[<ffffffff8109536a>] ? save_trace+0x4e/0xba
[<ffffffff81097e7b>] lock_acquire+0xee/0x12e
[<ffffffff811a7d08>] ? sysfs_notify_dirent+0x2c/0x75
[<ffffffff811a7d08>] ? sysfs_notify_dirent+0x2c/0x75
[<ffffffff813eb86a>] ? md_safemode_timeout+0x0/0x70
[<ffffffff814fc99f>] _spin_lock+0x45/0x8e
[<ffffffff811a7d08>] ? sysfs_notify_dirent+0x2c/0x75
[<ffffffff811a7d08>] sysfs_notify_dirent+0x2c/0x75
[<ffffffff813eb8b3>] md_safemode_timeout+0x49/0x70
[<ffffffff8107239e>] run_timer_softirq+0x1fc/0x2a3
[<ffffffff8107233a>] ? run_timer_softirq+0x198/0x2a3
[<ffffffff8106c8df>] __do_softirq+0xf6/0x1f0
[<ffffffff8101422c>] call_softirq+0x1c/0x30
[<ffffffff81015d77>] do_softirq+0x5f/0xd7
[<ffffffff8106c1f6>] irq_exit+0x66/0xbc
[<ffffffff814fc0ec>] ? trace_hardirqs_off_thunk+0x3a/0x3c
[<ffffffff81015406>] do_IRQ+0xb8/0xe5
[<ffffffff810139d3>] ret_from_intr+0x0/0x16
<EOI> [<ffffffff814fc71c>] ? _spin_unlock_irqrestore+0x5f/0x7f
[<ffffffff813580a6>] ? scsi_dispatch_cmd+0x1d4/0x25f
[<ffffffff8135f095>] ? scsi_request_fn+0x483/0x4c4
[<ffffffff8125c00e>] ? __blk_run_queue+0x54/0x9a
[<ffffffff8126e727>] ? cfq_insert_request+0x26c/0x3d4
[<ffffffff81253d83>] ? elv_insert+0x120/0x1e0
[<ffffffff81253eea>] ? __elv_add_request+0xa7/0xc2
[<ffffffff8125ce11>] ? __make_request+0x35e/0x3f1
[<ffffffff81259e20>] ? generic_make_request+0x29e/0x2fc
[<ffffffffa00db8af>] ? sync_request+0x833/0x8b2 [raid10]
[<ffffffff813f1104>] ? md_do_sync+0x756/0xb4f
[<ffffffff8101aa23>] ? native_sched_clock+0x2d/0x62
[<ffffffff813f1dbc>] ? md_thread+0x100/0x132
[<ffffffff813f1cbc>] ? md_thread+0x0/0x132
[<ffffffff810816f5>] ? kthread+0xa5/0xad
[<ffffffff8101412a>] ? child_rip+0xa/0x20
[<ffffffff81013a90>] ? restore_args+0x0/0x30
[<ffffffff81081650>] ? kthread+0x0/0xad
[<ffffffff81014120>] ? child_rip+0x0/0x20
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 2.6.31rc5 RAID10 lockdep report - sysfs_nofity_dirent locking issue
2009-08-04 14:14 2.6.31rc5 RAID10 lockdep report Dave Jones
@ 2009-08-04 21:13 ` NeilBrown
2009-08-04 21:24 ` Greg KH
0 siblings, 1 reply; 7+ messages in thread
From: NeilBrown @ 2009-08-04 21:13 UTC (permalink / raw)
To: Dave Jones, Linux Kernel, neilb; +Cc: gregkh
[-- Attachment #1: Type: text/plain, Size: 4415 bytes --]
On Wed, August 5, 2009 12:14 am, Dave Jones wrote:
> Report from a user we received today ..
> (https://bugzilla.redhat.com/show_bug.cgi?id=515471)
>
> Dave
>
>
> =================================
> [ INFO: inconsistent lock state ]
> 2.6.31-0.118.rc5.fc12.x86_64 #1
> ---------------------------------
> inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
> md126_resync/475 [HC0[0]:SC1[1]:HE1:SE0] takes:
> (sysfs_open_dirent_lock){+.?...}, at: [<ffffffff811a7d08>]
> sysfs_notify_dirent+0x2c/0x75
So the problem is that sysfs_open_dirent_lock is sometimes called
with SOFTIRQs enabled, and sometimes called in a SOFTIRQ and this
can lead to a deadlock.
The call from in a softirq is in sysfs_notify_dirent which is a very
small atomic function which I would certainly like to be able to
call from any context.
So the fix would be to use spin_lock_irq on the sysfs_open_dirent_lock ??
Or should it be spin_lock_bh ??
Attached is a suggestion for a patch.
Greg: does that look right? If so I'll add a changelog entry and
submit it properly (after at least a compile test...)
Thanks,
NeilBrown
> {SOFTIRQ-ON-W} state was registered at:
> [<ffffffff81097468>] __lock_acquire+0x2e9/0xc0e
> [<ffffffff81097e7b>] lock_acquire+0xee/0x12e
> [<ffffffff814fc99f>] _spin_lock+0x45/0x8e
> [<ffffffff811a70e0>] sysfs_open_file+0x18f/0x295
> [<ffffffff8113fba1>] __dentry_open+0x197/0x316
> [<ffffffff8113fe20>] nameidata_to_filp+0x51/0x76
> [<ffffffff8114e900>] do_filp_open+0x516/0x9d8
> [<ffffffff8113f889>] do_sys_open+0x71/0x131
> [<ffffffff8113f9b6>] sys_open+0x33/0x49
> [<ffffffff81012f42>] system_call_fastpath+0x16/0x1b
> [<ffffffffffffffff>] 0xffffffffffffffff
> irq event stamp: 12243532
> hardirqs last enabled at (12243532): [<ffffffff814fc69b>]
> _spin_unlock_irq+0x3f/0x61
> hardirqs last disabled at (12243531): [<ffffffff814fcaa9>]
> _spin_lock_irq+0x2e/0x9a
> softirqs last enabled at (12243378): [<ffffffff8106c9ad>]
> __do_softirq+0x1c4/0x1f0
> softirqs last disabled at (12243471): [<ffffffff8101422c>]
> call_softirq+0x1c/0x30
>
> other info that might help us debug this:
> 1 lock held by md126_resync/475:
> #0: (&new->safemode_timer){+.-...}, at: [<ffffffff8107233a>]
> run_timer_softirq+0x198/0x2a3
>
> stack backtrace:
> Pid: 475, comm: md126_resync Not tainted 2.6.31-0.118.rc5.fc12.x86_64 #1
> Call Trace:
> <IRQ> [<ffffffff81095f60>] valid_state+0x187/0x1ae
> [<ffffffff810969f1>] ? check_usage_forwards+0x0/0x79
> [<ffffffff810960b0>] mark_lock+0x129/0x253
> [<ffffffff810973f4>] __lock_acquire+0x275/0xc0e
> [<ffffffff8109536a>] ? save_trace+0x4e/0xba
> [<ffffffff81097e7b>] lock_acquire+0xee/0x12e
> [<ffffffff811a7d08>] ? sysfs_notify_dirent+0x2c/0x75
> [<ffffffff811a7d08>] ? sysfs_notify_dirent+0x2c/0x75
> [<ffffffff813eb86a>] ? md_safemode_timeout+0x0/0x70
> [<ffffffff814fc99f>] _spin_lock+0x45/0x8e
> [<ffffffff811a7d08>] ? sysfs_notify_dirent+0x2c/0x75
> [<ffffffff811a7d08>] sysfs_notify_dirent+0x2c/0x75
> [<ffffffff813eb8b3>] md_safemode_timeout+0x49/0x70
> [<ffffffff8107239e>] run_timer_softirq+0x1fc/0x2a3
> [<ffffffff8107233a>] ? run_timer_softirq+0x198/0x2a3
> [<ffffffff8106c8df>] __do_softirq+0xf6/0x1f0
> [<ffffffff8101422c>] call_softirq+0x1c/0x30
> [<ffffffff81015d77>] do_softirq+0x5f/0xd7
> [<ffffffff8106c1f6>] irq_exit+0x66/0xbc
> [<ffffffff814fc0ec>] ? trace_hardirqs_off_thunk+0x3a/0x3c
> [<ffffffff81015406>] do_IRQ+0xb8/0xe5
> [<ffffffff810139d3>] ret_from_intr+0x0/0x16
> <EOI> [<ffffffff814fc71c>] ? _spin_unlock_irqrestore+0x5f/0x7f
> [<ffffffff813580a6>] ? scsi_dispatch_cmd+0x1d4/0x25f
> [<ffffffff8135f095>] ? scsi_request_fn+0x483/0x4c4
> [<ffffffff8125c00e>] ? __blk_run_queue+0x54/0x9a
> [<ffffffff8126e727>] ? cfq_insert_request+0x26c/0x3d4
> [<ffffffff81253d83>] ? elv_insert+0x120/0x1e0
> [<ffffffff81253eea>] ? __elv_add_request+0xa7/0xc2
> [<ffffffff8125ce11>] ? __make_request+0x35e/0x3f1
> [<ffffffff81259e20>] ? generic_make_request+0x29e/0x2fc
> [<ffffffffa00db8af>] ? sync_request+0x833/0x8b2 [raid10]
> [<ffffffff813f1104>] ? md_do_sync+0x756/0xb4f
> [<ffffffff8101aa23>] ? native_sched_clock+0x2d/0x62
> [<ffffffff813f1dbc>] ? md_thread+0x100/0x132
> [<ffffffff813f1cbc>] ? md_thread+0x0/0x132
> [<ffffffff810816f5>] ? kthread+0xa5/0xad
> [<ffffffff8101412a>] ? child_rip+0xa/0x20
> [<ffffffff81013a90>] ? restore_args+0x0/0x30
> [<ffffffff81081650>] ? kthread+0x0/0xad
> [<ffffffff81014120>] ? child_rip+0x0/0x20
>
[-- Attachment #2: diff --]
[-- Type: application/octet-stream, Size: 1808 bytes --]
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 561a9c0..f5ea468 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -268,7 +268,7 @@ static int sysfs_get_open_dirent(struct sysfs_dirent *sd,
struct sysfs_open_dirent *od, *new_od = NULL;
retry:
- spin_lock(&sysfs_open_dirent_lock);
+ spin_lock_irq(&sysfs_open_dirent_lock);
if (!sd->s_attr.open && new_od) {
sd->s_attr.open = new_od;
@@ -281,7 +281,7 @@ static int sysfs_get_open_dirent(struct sysfs_dirent *sd,
list_add_tail(&buffer->list, &od->buffers);
}
- spin_unlock(&sysfs_open_dirent_lock);
+ spin_unlock_irq(&sysfs_open_dirent_lock);
if (od) {
kfree(new_od);
@@ -315,8 +315,9 @@ static void sysfs_put_open_dirent(struct sysfs_dirent *sd,
struct sysfs_buffer *buffer)
{
struct sysfs_open_dirent *od = sd->s_attr.open;
+ unsigned long flags;
- spin_lock(&sysfs_open_dirent_lock);
+ spin_lock_irqsave(&sysfs_open_dirent_lock, flags);
list_del(&buffer->list);
if (atomic_dec_and_test(&od->refcnt))
@@ -324,7 +325,7 @@ static void sysfs_put_open_dirent(struct sysfs_dirent *sd,
else
od = NULL;
- spin_unlock(&sysfs_open_dirent_lock);
+ spin_unlock_irqrestore(&sysfs_open_dirent_lock, flags);
kfree(od);
}
@@ -456,8 +457,9 @@ static unsigned int sysfs_poll(struct file *filp, poll_table *wait)
void sysfs_notify_dirent(struct sysfs_dirent *sd)
{
struct sysfs_open_dirent *od;
+ unsigned long flags;
- spin_lock(&sysfs_open_dirent_lock);
+ spin_lock_irqsave(&sysfs_open_dirent_lock, flags);
od = sd->s_attr.open;
if (od) {
@@ -465,7 +467,7 @@ void sysfs_notify_dirent(struct sysfs_dirent *sd)
wake_up_interruptible(&od->poll);
}
- spin_unlock(&sysfs_open_dirent_lock);
+ spin_unlock_irqrestore(&sysfs_open_dirent_lock, flags);
}
EXPORT_SYMBOL_GPL(sysfs_notify_dirent);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: 2.6.31rc5 RAID10 lockdep report - sysfs_nofity_dirent locking issue
2009-08-04 21:13 ` 2.6.31rc5 RAID10 lockdep report - sysfs_nofity_dirent locking issue NeilBrown
@ 2009-08-04 21:24 ` Greg KH
2009-08-06 5:43 ` Neil Brown
0 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2009-08-04 21:24 UTC (permalink / raw)
To: NeilBrown; +Cc: Dave Jones, Linux Kernel
On Wed, Aug 05, 2009 at 07:13:08AM +1000, NeilBrown wrote:
> On Wed, August 5, 2009 12:14 am, Dave Jones wrote:
> > Report from a user we received today ..
> > (https://bugzilla.redhat.com/show_bug.cgi?id=515471)
> >
> > Dave
> >
> >
> > =================================
> > [ INFO: inconsistent lock state ]
> > 2.6.31-0.118.rc5.fc12.x86_64 #1
> > ---------------------------------
> > inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
> > md126_resync/475 [HC0[0]:SC1[1]:HE1:SE0] takes:
> > (sysfs_open_dirent_lock){+.?...}, at: [<ffffffff811a7d08>]
> > sysfs_notify_dirent+0x2c/0x75
>
> So the problem is that sysfs_open_dirent_lock is sometimes called
> with SOFTIRQs enabled, and sometimes called in a SOFTIRQ and this
> can lead to a deadlock.
>
> The call from in a softirq is in sysfs_notify_dirent which is a very
> small atomic function which I would certainly like to be able to
> call from any context.
>
> So the fix would be to use spin_lock_irq on the sysfs_open_dirent_lock ??
> Or should it be spin_lock_bh ??
>
> Attached is a suggestion for a patch.
>
> Greg: does that look right? If so I'll add a changelog entry and
> submit it properly (after at least a compile test...)
Yes, it looks correct, if it passes your tests :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 2.6.31rc5 RAID10 lockdep report - sysfs_nofity_dirent locking issue
2009-08-04 21:24 ` Greg KH
@ 2009-08-06 5:43 ` Neil Brown
2009-09-15 22:12 ` Dan Williams
0 siblings, 1 reply; 7+ messages in thread
From: Neil Brown @ 2009-08-06 5:43 UTC (permalink / raw)
To: Greg KH; +Cc: NeilBrown, Dave Jones, Linux Kernel
On Tuesday August 4, gregkh@suse.de wrote:
> >
> > Greg: does that look right? If so I'll add a changelog entry and
> > submit it properly (after at least a compile test...)
>
> Yes, it looks correct, if it passes your tests :)
Thanks.
I've tested it and documented it now and so am happy to submit it.
Thanks,
NeilBrown
From: NeilBrown <neilb@suse.de>
Date: Thu, 6 Aug 2009 15:42:08 +1000
Subject: [PATCH] Allow sysfs_notify_dirent to be called from interrupt context.
sysfs_notify_dirent is a simple atomic operation that can be used to
alert user-space that new data can be read from a sysfs attribute.
Unfortunately is cannot currently be called from non-process context
because of it's use of spin_lock which is sometimes taken with
interrupt enabled.
So change all lockers of sysfs_open_dirent_lock to disable interrupts,
thus making sysfs_notify_dirent safe to be called from non-process
context (as drivers/md does in md_safemode_timeout).
sysfs_get_open_dirent is (documented as being) only called from
process context, so it uses spin_lock_irq. Other places
use spin_lock_irqsave.
The usage for sysfs_notify_dirent in md_safemode_timeout was
introduced in 2.6.28, so this patch is suitable for that and more
recent kernels.
Cc: stable@kernel.org
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/sysfs/file.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 561a9c0..f5ea468 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -268,7 +268,7 @@ static int sysfs_get_open_dirent(struct sysfs_dirent *sd,
struct sysfs_open_dirent *od, *new_od = NULL;
retry:
- spin_lock(&sysfs_open_dirent_lock);
+ spin_lock_irq(&sysfs_open_dirent_lock);
if (!sd->s_attr.open && new_od) {
sd->s_attr.open = new_od;
@@ -281,7 +281,7 @@ static int sysfs_get_open_dirent(struct sysfs_dirent *sd,
list_add_tail(&buffer->list, &od->buffers);
}
- spin_unlock(&sysfs_open_dirent_lock);
+ spin_unlock_irq(&sysfs_open_dirent_lock);
if (od) {
kfree(new_od);
@@ -315,8 +315,9 @@ static void sysfs_put_open_dirent(struct sysfs_dirent *sd,
struct sysfs_buffer *buffer)
{
struct sysfs_open_dirent *od = sd->s_attr.open;
+ unsigned long flags;
- spin_lock(&sysfs_open_dirent_lock);
+ spin_lock_irqsave(&sysfs_open_dirent_lock, flags);
list_del(&buffer->list);
if (atomic_dec_and_test(&od->refcnt))
@@ -324,7 +325,7 @@ static void sysfs_put_open_dirent(struct sysfs_dirent *sd,
else
od = NULL;
- spin_unlock(&sysfs_open_dirent_lock);
+ spin_unlock_irqrestore(&sysfs_open_dirent_lock, flags);
kfree(od);
}
@@ -456,8 +457,9 @@ static unsigned int sysfs_poll(struct file *filp, poll_table *wait)
void sysfs_notify_dirent(struct sysfs_dirent *sd)
{
struct sysfs_open_dirent *od;
+ unsigned long flags;
- spin_lock(&sysfs_open_dirent_lock);
+ spin_lock_irqsave(&sysfs_open_dirent_lock, flags);
od = sd->s_attr.open;
if (od) {
@@ -465,7 +467,7 @@ void sysfs_notify_dirent(struct sysfs_dirent *sd)
wake_up_interruptible(&od->poll);
}
- spin_unlock(&sysfs_open_dirent_lock);
+ spin_unlock_irqrestore(&sysfs_open_dirent_lock, flags);
}
EXPORT_SYMBOL_GPL(sysfs_notify_dirent);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: 2.6.31rc5 RAID10 lockdep report - sysfs_nofity_dirent locking issue
2009-08-06 5:43 ` Neil Brown
@ 2009-09-15 22:12 ` Dan Williams
2009-09-15 22:25 ` Greg KH
0 siblings, 1 reply; 7+ messages in thread
From: Dan Williams @ 2009-09-15 22:12 UTC (permalink / raw)
To: Greg KH; +Cc: Neil Brown, Dave Jones, Linux Kernel, Hans de Goede
On Wed, Aug 5, 2009 at 10:43 PM, Neil Brown <neilb@suse.de> wrote:
> On Tuesday August 4, gregkh@suse.de wrote:
>> >
>> > Greg: does that look right? If so I'll add a changelog entry and
>> > submit it properly (after at least a compile test...)
>>
>> Yes, it looks correct, if it passes your tests :)
>
> Thanks.
> I've tested it and documented it now and so am happy to submit it.
> Thanks,
> NeilBrown
>
>
> From: NeilBrown <neilb@suse.de>
> Date: Thu, 6 Aug 2009 15:42:08 +1000
> Subject: [PATCH] Allow sysfs_notify_dirent to be called from interrupt context.
>
> sysfs_notify_dirent is a simple atomic operation that can be used to
> alert user-space that new data can be read from a sysfs attribute.
>
> Unfortunately is cannot currently be called from non-process context
> because of it's use of spin_lock which is sometimes taken with
> interrupt enabled.
>
> So change all lockers of sysfs_open_dirent_lock to disable interrupts,
> thus making sysfs_notify_dirent safe to be called from non-process
> context (as drivers/md does in md_safemode_timeout).
>
> sysfs_get_open_dirent is (documented as being) only called from
> process context, so it uses spin_lock_irq. Other places
> use spin_lock_irqsave.
>
> The usage for sysfs_notify_dirent in md_safemode_timeout was
> introduced in 2.6.28, so this patch is suitable for that and more
> recent kernels.
>
> Cc: stable@kernel.org
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
Greg?
Looks like this never made it upstream, and now Hans is hitting this
in his tests.
Thanks,
Dan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 2.6.31rc5 RAID10 lockdep report - sysfs_nofity_dirent locking issue
2009-09-15 22:12 ` Dan Williams
@ 2009-09-15 22:25 ` Greg KH
2009-09-15 23:05 ` Dan Williams
0 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2009-09-15 22:25 UTC (permalink / raw)
To: Dan Williams; +Cc: Neil Brown, Dave Jones, Linux Kernel, Hans de Goede
On Tue, Sep 15, 2009 at 03:12:07PM -0700, Dan Williams wrote:
> On Wed, Aug 5, 2009 at 10:43 PM, Neil Brown <neilb@suse.de> wrote:
> > On Tuesday August 4, gregkh@suse.de wrote:
> >> >
> >> > Greg: does that look right? If so I'll add a changelog entry and
> >> > submit it properly (after at least a compile test...)
> >>
> >> Yes, it looks correct, if it passes your tests :)
> >
> > Thanks.
> > I've tested it and documented it now and so am happy to submit it.
> > Thanks,
> > NeilBrown
> >
> >
> > From: NeilBrown <neilb@suse.de>
> > Date: Thu, 6 Aug 2009 15:42:08 +1000
> > Subject: [PATCH] Allow sysfs_notify_dirent to be called from interrupt context.
> >
> > sysfs_notify_dirent is a simple atomic operation that can be used to
> > alert user-space that new data can be read from a sysfs attribute.
> >
> > Unfortunately is cannot currently be called from non-process context
> > because of it's use of spin_lock which is sometimes taken with
> > interrupt enabled.
> >
> > So change all lockers of sysfs_open_dirent_lock to disable interrupts,
> > thus making sysfs_notify_dirent safe to be called from non-process
> > context (as drivers/md does in md_safemode_timeout).
> >
> > sysfs_get_open_dirent is (documented as being) only called from
> > process context, so it uses spin_lock_irq. Other places
> > use spin_lock_irqsave.
> >
> > The usage for sysfs_notify_dirent in md_safemode_timeout was
> > introduced in 2.6.28, so this patch is suitable for that and more
> > recent kernels.
> >
> > Cc: stable@kernel.org
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
>
> Greg?
>
> Looks like this never made it upstream, and now Hans is hitting this
> in his tests.
Ok, I don't have this in my queue, so someone needs to resubmit it and
tell me what kernel trees it should be applicable for.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 2.6.31rc5 RAID10 lockdep report - sysfs_nofity_dirent locking issue
2009-09-15 22:25 ` Greg KH
@ 2009-09-15 23:05 ` Dan Williams
0 siblings, 0 replies; 7+ messages in thread
From: Dan Williams @ 2009-09-15 23:05 UTC (permalink / raw)
To: Greg KH; +Cc: Neil Brown, Dave Jones, Linux Kernel, Hans de Goede
>From 1ee8b5bfeb2de765d3b2acbaf5357c4c0eb65dbe Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@suse.de>
Date: Thu, 6 Aug 2009 15:42:08 +1000
Subject: [PATCH] Allow sysfs_notify_dirent to be called from interrupt context.
sysfs_notify_dirent is a simple atomic operation that can be used to
alert user-space that new data can be read from a sysfs attribute.
Unfortunately it cannot currently be called from non-process context
because of its use of spin_lock which is sometimes taken with
interrupts enabled.
So change all lockers of sysfs_open_dirent_lock to disable interrupts,
thus making sysfs_notify_dirent safe to be called from non-process
context (as drivers/md does in md_safemode_timeout).
sysfs_get_open_dirent is (documented as being) only called from
process context, so it uses spin_lock_irq. Other places
use spin_lock_irqsave.
The usage for sysfs_notify_dirent in md_safemode_timeout was
introduced in 2.6.28, so this patch is suitable for that and more
recent kernels.
Cc: <stable@kernel.org>
Reported-by: Joel Andres Granados <jgranado@redhat.com>
Signed-off-by: NeilBrown <neilb@suse.de>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
> > Greg?
> >
> > Looks like this never made it upstream, and now Hans is hitting this
> > in his tests.
>
> Ok, I don't have this in my queue, so someone needs to resubmit it and
> tell me what kernel trees it should be applicable for.
As mentioned in the commit log should be applicable back to 2.6.28.
Thanks,
Dan
fs/sysfs/file.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 561a9c0..f5ea468 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -268,7 +268,7 @@ static int sysfs_get_open_dirent(struct sysfs_dirent *sd,
struct sysfs_open_dirent *od, *new_od = NULL;
retry:
- spin_lock(&sysfs_open_dirent_lock);
+ spin_lock_irq(&sysfs_open_dirent_lock);
if (!sd->s_attr.open && new_od) {
sd->s_attr.open = new_od;
@@ -281,7 +281,7 @@ static int sysfs_get_open_dirent(struct sysfs_dirent *sd,
list_add_tail(&buffer->list, &od->buffers);
}
- spin_unlock(&sysfs_open_dirent_lock);
+ spin_unlock_irq(&sysfs_open_dirent_lock);
if (od) {
kfree(new_od);
@@ -315,8 +315,9 @@ static void sysfs_put_open_dirent(struct sysfs_dirent *sd,
struct sysfs_buffer *buffer)
{
struct sysfs_open_dirent *od = sd->s_attr.open;
+ unsigned long flags;
- spin_lock(&sysfs_open_dirent_lock);
+ spin_lock_irqsave(&sysfs_open_dirent_lock, flags);
list_del(&buffer->list);
if (atomic_dec_and_test(&od->refcnt))
@@ -324,7 +325,7 @@ static void sysfs_put_open_dirent(struct sysfs_dirent *sd,
else
od = NULL;
- spin_unlock(&sysfs_open_dirent_lock);
+ spin_unlock_irqrestore(&sysfs_open_dirent_lock, flags);
kfree(od);
}
@@ -456,8 +457,9 @@ static unsigned int sysfs_poll(struct file *filp, poll_table *wait)
void sysfs_notify_dirent(struct sysfs_dirent *sd)
{
struct sysfs_open_dirent *od;
+ unsigned long flags;
- spin_lock(&sysfs_open_dirent_lock);
+ spin_lock_irqsave(&sysfs_open_dirent_lock, flags);
od = sd->s_attr.open;
if (od) {
@@ -465,7 +467,7 @@ void sysfs_notify_dirent(struct sysfs_dirent *sd)
wake_up_interruptible(&od->poll);
}
- spin_unlock(&sysfs_open_dirent_lock);
+ spin_unlock_irqrestore(&sysfs_open_dirent_lock, flags);
}
EXPORT_SYMBOL_GPL(sysfs_notify_dirent);
--
1.6.0.6
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-09-15 23:05 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-04 14:14 2.6.31rc5 RAID10 lockdep report Dave Jones
2009-08-04 21:13 ` 2.6.31rc5 RAID10 lockdep report - sysfs_nofity_dirent locking issue NeilBrown
2009-08-04 21:24 ` Greg KH
2009-08-06 5:43 ` Neil Brown
2009-09-15 22:12 ` Dan Williams
2009-09-15 22:25 ` Greg KH
2009-09-15 23:05 ` Dan Williams
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.