Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
On Wed, Apr 11, 2007 at 01:15:15PM +0900, Tejun Heo wrote: > [PATCH] sysfs: implement sysfs_dirent active reference and immediate > disconnect For some reason, this doesn't apply on top of your patches: Applying patch sysfs-implement-sysfs_dirent-active-reference-and-immediate-disconnect.patch patching file fs/sysfs/dir.c patching file fs/sysfs/sysfs.h patching file fs/sysfs/bin.c patching file fs/sysfs/file.c Hunk #1 succeeded at 96 (offset 8 lines). Hunk #2 succeeded at 107 (offset 8 lines). Hunk #3 succeeded at 240 (offset 8 lines). Hunk #4 FAILED at 300. Hunk #5 succeeded at 336 (offset 14 lines). Hunk #6 succeeded at 368 (offset 14 lines). Hunk #7 succeeded at 435 (offset 14 lines). 1 out of 7 hunks FAILED -- rejects in file fs/sysfs/file.c patching file fs/sysfs/inode.c So I'll only apply the first 11 for now. Can you respin the last 3 and send them to me again? thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
On Thu, Apr 12, 2007 at 12:18:15AM -0700, Greg KH wrote: > On Wed, Apr 11, 2007 at 01:15:15PM +0900, Tejun Heo wrote: > > [PATCH] sysfs: implement sysfs_dirent active reference and immediate > > disconnect > > For some reason, this doesn't apply on top of your patches: > > Applying patch > sysfs-implement-sysfs_dirent-active-reference-and-immediate-disconnect.patch > patching file fs/sysfs/dir.c > patching file fs/sysfs/sysfs.h > patching file fs/sysfs/bin.c > patching file fs/sysfs/file.c > Hunk #1 succeeded at 96 (offset 8 lines). > Hunk #2 succeeded at 107 (offset 8 lines). > Hunk #3 succeeded at 240 (offset 8 lines). > Hunk #4 FAILED at 300. > Hunk #5 succeeded at 336 (offset 14 lines). > Hunk #6 succeeded at 368 (offset 14 lines). > Hunk #7 succeeded at 435 (offset 14 lines). > 1 out of 7 hunks FAILED -- rejects in file fs/sysfs/file.c > patching file fs/sysfs/inode.c > > So I'll only apply the first 11 for now. Can you respin the last 3 and > send them to me again? Oh nevermind, this was my fault, it was due to something else of mine that isn't in mainline, sorry for the noise... greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
On Wed, Apr 11, 2007 at 01:15:15PM +0900, Tejun Heo wrote: [PATCH] sysfs: implement sysfs_dirent active reference and immediate disconnect For some reason, this doesn't apply on top of your patches: Applying patch sysfs-implement-sysfs_dirent-active-reference-and-immediate-disconnect.patch patching file fs/sysfs/dir.c patching file fs/sysfs/sysfs.h patching file fs/sysfs/bin.c patching file fs/sysfs/file.c Hunk #1 succeeded at 96 (offset 8 lines). Hunk #2 succeeded at 107 (offset 8 lines). Hunk #3 succeeded at 240 (offset 8 lines). Hunk #4 FAILED at 300. Hunk #5 succeeded at 336 (offset 14 lines). Hunk #6 succeeded at 368 (offset 14 lines). Hunk #7 succeeded at 435 (offset 14 lines). 1 out of 7 hunks FAILED -- rejects in file fs/sysfs/file.c patching file fs/sysfs/inode.c So I'll only apply the first 11 for now. Can you respin the last 3 and send them to me again? thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
On Thu, Apr 12, 2007 at 12:18:15AM -0700, Greg KH wrote: On Wed, Apr 11, 2007 at 01:15:15PM +0900, Tejun Heo wrote: [PATCH] sysfs: implement sysfs_dirent active reference and immediate disconnect For some reason, this doesn't apply on top of your patches: Applying patch sysfs-implement-sysfs_dirent-active-reference-and-immediate-disconnect.patch patching file fs/sysfs/dir.c patching file fs/sysfs/sysfs.h patching file fs/sysfs/bin.c patching file fs/sysfs/file.c Hunk #1 succeeded at 96 (offset 8 lines). Hunk #2 succeeded at 107 (offset 8 lines). Hunk #3 succeeded at 240 (offset 8 lines). Hunk #4 FAILED at 300. Hunk #5 succeeded at 336 (offset 14 lines). Hunk #6 succeeded at 368 (offset 14 lines). Hunk #7 succeeded at 435 (offset 14 lines). 1 out of 7 hunks FAILED -- rejects in file fs/sysfs/file.c patching file fs/sysfs/inode.c So I'll only apply the first 11 for now. Can you respin the last 3 and send them to me again? Oh nevermind, this was my fault, it was due to something else of mine that isn't in mainline, sorry for the noise... greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
On Wed, 11 Apr 2007 19:13:59 +0900,
Tejun Heo <[EMAIL PROTECTED]> wrote:
> Does this patch fix the problem?
>
> Index: work/fs/sysfs/dir.c
> ===
> --- work.orig/fs/sysfs/dir.c 2007-04-11 19:12:02.0 +0900
> +++ work/fs/sysfs/dir.c 2007-04-11 19:12:12.0 +0900
> @@ -26,7 +26,8 @@ void release_sysfs_dirent(struct sysfs_d
>* locked. If @sd is cursor for directory walk or being
>* released prematurely, s_active has no reader or writer.
>*/
> - down_write_trylock(>s_active);
> + if (!down_write_trylock(>s_active))
> + rwsem_acquire(>s_active.dep_map, 0, 0, _RET_IP_);
> up_write(>s_active);
>
> if (sd->s_type & SYSFS_KOBJ_LINK)
> Index: work/fs/sysfs/sysfs.h
> ===
> --- work.orig/fs/sysfs/sysfs.h2007-04-11 19:12:02.0 +0900
> +++ work/fs/sysfs/sysfs.h 2007-04-11 19:12:03.0 +0900
> @@ -171,6 +171,7 @@ static inline void sysfs_put_active_two(
> static inline void sysfs_deactivate(struct sysfs_dirent *sd)
> {
> down_write(>s_active);
> + rwsem_release(>s_active.dep_map, 1, _RET_IP_);
> }
>
> static inline int sysfs_is_shadowed_inode(struct inode *inode)
This seems to work fine now, thanks.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
On Wed, Apr 11, 2007 at 06:26:42PM +0900, Tejun Heo wrote:
> Hello,
>
> Cornelia Huck wrote:
> > ===
> > [ INFO: possible circular locking dependency detected ]
> > 2.6.21-rc6-ge666c753-dirty #54
> > ---
> > kslowcrw/64 is trying to acquire lock:
> > (>reg_mutex){--..}, at: [<004233b2>] mutex_lock+0x3e/0x4c
> >
> > but task is already holding lock:
> > (>s_active){}, at: [<00209578>] remove_dir+0xec/0x144
> >
> > which lock already depends on the new lock.
>
> This is probably because s_active has different write locked and
> unlocked by different threads but it doesn't tell lockdep about it.
> I'll read lockdep docs and update it.
Does this patch fix the problem?
Index: work/fs/sysfs/dir.c
===
--- work.orig/fs/sysfs/dir.c2007-04-11 19:12:02.0 +0900
+++ work/fs/sysfs/dir.c 2007-04-11 19:12:12.0 +0900
@@ -26,7 +26,8 @@ void release_sysfs_dirent(struct sysfs_d
* locked. If @sd is cursor for directory walk or being
* released prematurely, s_active has no reader or writer.
*/
- down_write_trylock(>s_active);
+ if (!down_write_trylock(>s_active))
+ rwsem_acquire(>s_active.dep_map, 0, 0, _RET_IP_);
up_write(>s_active);
if (sd->s_type & SYSFS_KOBJ_LINK)
Index: work/fs/sysfs/sysfs.h
===
--- work.orig/fs/sysfs/sysfs.h 2007-04-11 19:12:02.0 +0900
+++ work/fs/sysfs/sysfs.h 2007-04-11 19:12:03.0 +0900
@@ -171,6 +171,7 @@ static inline void sysfs_put_active_two(
static inline void sysfs_deactivate(struct sysfs_dirent *sd)
{
down_write(>s_active);
+ rwsem_release(>s_active.dep_map, 1, _RET_IP_);
}
static inline int sysfs_is_shadowed_inode(struct inode *inode)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
Tejun Heo wrote:
> Hello,
>
> Cornelia Huck wrote:
>> ===
>> [ INFO: possible circular locking dependency detected ]
>> 2.6.21-rc6-ge666c753-dirty #54
>> ---
>> kslowcrw/64 is trying to acquire lock:
>> (>reg_mutex){--..}, at: [<004233b2>] mutex_lock+0x3e/0x4c
>>
>> but task is already holding lock:
>> (>s_active){}, at: [<00209578>] remove_dir+0xec/0x144
>>
>> which lock already depends on the new lock.
>
> This is probably because s_active has different write locked and
> unlocked by different threads but it doesn't tell lockdep about it.
s/has different/is/
Sorry.
--
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
Hello,
Cornelia Huck wrote:
> ===
> [ INFO: possible circular locking dependency detected ]
> 2.6.21-rc6-ge666c753-dirty #54
> ---
> kslowcrw/64 is trying to acquire lock:
> (>reg_mutex){--..}, at: [<004233b2>] mutex_lock+0x3e/0x4c
>
> but task is already holding lock:
> (>s_active){}, at: [<00209578>] remove_dir+0xec/0x144
>
> which lock already depends on the new lock.
This is probably because s_active has different write locked and
unlocked by different threads but it doesn't tell lockdep about it.
I'll read lockdep docs and update it.
Thanks for testing.
--
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
On Wed, 11 Apr 2007 13:15:15 +0900,
Tejun Heo <[EMAIL PROTECTED]> wrote:
> Cornelia, this should fix the problem you reported. It's caused by
> doing up_write() on sysfs_dirent which is used for readdir cursor
> which is not deactivated before being released.
> release_sysfs_dirent() is udpated such that it does
> down_write_trylock() on s_active before doing up_write(). This also
> covers error paths where allocated sysfs_dirent is put due to errors
> during initialization.
I can confirm that this fixes the problem I saw earlier. However, I now
have a new one:
1. Define virtunal ctcs 7000 and 7001: This
creates /sys/bus/ccw/devices/{0.0.7000,0.0.7001}.
2. Do echo 0.0.7000,0.0.7001 > /sys/bus/ccwgroup/drivers/ctc/group.
This creates /sys/bus/ccwgroup/devices/0.0.7000, which has the two ccw
devices from above as slave devices.
3. Detach virtual ctcs 7000 and 7001. This causes the subchannel
beneath 0.0.7000 resp. 0.0.7001 to be unregistered (from a workqueue)
which will in turn trigger an unregistration of ccw device 0.0.7000,
ccw group device 0.0.7000 and ccw device 0.0.7001. Now lockdep is
unhappy:
crw_info : CRW reports slct=0, oflw=0, chn=0, rsc=3, anc=1, erc=4, rsid=F
crw_info : CRW reports slct=0, oflw=0, chn=0, rsc=3, anc=1, erc=4, rsid=10
DEV: Unregistering device. ID = '0.0.000f'
bus css: remove device 0.0.000f
DEV: Unregistering device. ID = '0.0.7000'
bus ccw: remove device 0.0.7000
DEV: Unregistering device. ID = '0.0.7000'
bus ccwgroup: remove device 0.0.7000
===
[ INFO: possible circular locking dependency detected ]
2.6.21-rc6-ge666c753-dirty #54
---
kslowcrw/64 is trying to acquire lock:
(>reg_mutex){--..}, at: [<004233b2>] mutex_lock+0x3e/0x4c
but task is already holding lock:
(>s_active){}, at: [<00209578>] remove_dir+0xec/0x144
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (>s_active){}:
[<00156998>] __lock_acquire+0x1008/0x1124
[<00156e9c>] lock_acquire+0x78/0xa8
[<0014fc96>] down_write+0x5a/0xa0
[<00206f0a>] sysfs_hash_and_remove+0x112/0x15c
[<002071a6>] sysfs_remove_file+0x32/0x40
[<002a76bc>] device_remove_file+0x44/0x5c
[<002a813a>] device_del+0x1ae/0x26c
[<002a8236>] device_unregister+0x3e/0x58
[<002f0bc0>] css_sch_device_unregister+0x3c/0x54
[<002f174a>] css_evaluate_subchannel+0x266/0x3b8
[<002f1976>] css_trigger_slow_path+0xda/0x154
[<00144a6c>] run_workqueue+0x174/0x220
[<00144d26>] worker_thread+0x116/0x164
[<0014ab9c>] kthread+0x158/0x160
[<00107c3a>] kernel_thread_starter+0x6/0xc
[<00107c34>] kernel_thread_starter+0x0/0xc
-> #0 (>reg_mutex){--..}:
[<0015671c>] __lock_acquire+0xd8c/0x1124
[<00156e9c>] lock_acquire+0x78/0xa8
[<00423070>] __mutex_lock_slowpath+0xbc/0x3c0
[<004233b2>] mutex_lock+0x3e/0x4c
[<002f0bb6>] css_sch_device_unregister+0x32/0x54
[<002f174a>] css_evaluate_subchannel+0x266/0x3b8
[<002f1976>] css_trigger_slow_path+0xda/0x154
[<00144a6c>] run_workqueue+0x174/0x220
[<00144d26>] worker_thread+0x116/0x164
[<0014ab9c>] kthread+0x158/0x160
[<00107c3a>] kernel_thread_starter+0x6/0xc
[<00107c34>] kernel_thread_starter+0x0/0xc
other info that might help us debug this:
1 lock held by kslowcrw/64:
#0: (>s_active){}, at: [<00209578>] remove_dir+0xec/0x144
stack backtrace:
0002 0002 1fd1ba50
1fd1b9c8 004bebf6 004bebf6 00103854
0001 00602cb0
1fd1ba10 1fd1b9b0 000e
0042a780 0010389e 1fd1b9b0 1fd1ba00
Call Trace:
([<001037f4>] show_trace+0xf8/0xfc)
[<001038ce>] show_stack+0xd6/0x100
[<00103926>] dump_stack+0x2e/0x3c
[<00154c6e>] print_circular_bug_tail+0x9e/0xb0
[<0015671c>] __lock_acquire+0xd8c/0x1124
[<00156e9c>] lock_acquire+0x78/0xa8
[<00423070>] __mutex_lock_slowpath+0xbc/0x3c0
[<004233b2>] mutex_lock+0x3e/0x4c
[<002f0bb6>] css_sch_device_unregister+0x32/0x54
[<002f174a>] css_evaluate_subchannel+0x266/0x3b8
[<002f1976>] css_trigger_slow_path+0xda/0x154
[<00144a6c>] run_workqueue+0x174/0x220
[<00144d26>] worker_thread+0x116/0x164
[<0014ab9c>] kthread+0x158/0x160
[<00107c3a>] kernel_thread_starter+0x6/0xc
[<00107c34>] kernel_thread_starter+0x0/0xc
INFO: lockdep
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
On Wed, 11 Apr 2007 13:15:15 +0900,
Tejun Heo [EMAIL PROTECTED] wrote:
Cornelia, this should fix the problem you reported. It's caused by
doing up_write() on sysfs_dirent which is used for readdir cursor
which is not deactivated before being released.
release_sysfs_dirent() is udpated such that it does
down_write_trylock() on s_active before doing up_write(). This also
covers error paths where allocated sysfs_dirent is put due to errors
during initialization.
I can confirm that this fixes the problem I saw earlier. However, I now
have a new one:
1. Define virtunal ctcs 7000 and 7001: This
creates /sys/bus/ccw/devices/{0.0.7000,0.0.7001}.
2. Do echo 0.0.7000,0.0.7001 /sys/bus/ccwgroup/drivers/ctc/group.
This creates /sys/bus/ccwgroup/devices/0.0.7000, which has the two ccw
devices from above as slave devices.
3. Detach virtual ctcs 7000 and 7001. This causes the subchannel
beneath 0.0.7000 resp. 0.0.7001 to be unregistered (from a workqueue)
which will in turn trigger an unregistration of ccw device 0.0.7000,
ccw group device 0.0.7000 and ccw device 0.0.7001. Now lockdep is
unhappy:
crw_info : CRW reports slct=0, oflw=0, chn=0, rsc=3, anc=1, erc=4, rsid=F
crw_info : CRW reports slct=0, oflw=0, chn=0, rsc=3, anc=1, erc=4, rsid=10
DEV: Unregistering device. ID = '0.0.000f'
bus css: remove device 0.0.000f
DEV: Unregistering device. ID = '0.0.7000'
bus ccw: remove device 0.0.7000
DEV: Unregistering device. ID = '0.0.7000'
bus ccwgroup: remove device 0.0.7000
===
[ INFO: possible circular locking dependency detected ]
2.6.21-rc6-ge666c753-dirty #54
---
kslowcrw/64 is trying to acquire lock:
(sch-reg_mutex){--..}, at: [004233b2] mutex_lock+0x3e/0x4c
but task is already holding lock:
(sd-s_active){}, at: [00209578] remove_dir+0xec/0x144
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
- #1 (sd-s_active){}:
[00156998] __lock_acquire+0x1008/0x1124
[00156e9c] lock_acquire+0x78/0xa8
[0014fc96] down_write+0x5a/0xa0
[00206f0a] sysfs_hash_and_remove+0x112/0x15c
[002071a6] sysfs_remove_file+0x32/0x40
[002a76bc] device_remove_file+0x44/0x5c
[002a813a] device_del+0x1ae/0x26c
[002a8236] device_unregister+0x3e/0x58
[002f0bc0] css_sch_device_unregister+0x3c/0x54
[002f174a] css_evaluate_subchannel+0x266/0x3b8
[002f1976] css_trigger_slow_path+0xda/0x154
[00144a6c] run_workqueue+0x174/0x220
[00144d26] worker_thread+0x116/0x164
[0014ab9c] kthread+0x158/0x160
[00107c3a] kernel_thread_starter+0x6/0xc
[00107c34] kernel_thread_starter+0x0/0xc
- #0 (sch-reg_mutex){--..}:
[0015671c] __lock_acquire+0xd8c/0x1124
[00156e9c] lock_acquire+0x78/0xa8
[00423070] __mutex_lock_slowpath+0xbc/0x3c0
[004233b2] mutex_lock+0x3e/0x4c
[002f0bb6] css_sch_device_unregister+0x32/0x54
[002f174a] css_evaluate_subchannel+0x266/0x3b8
[002f1976] css_trigger_slow_path+0xda/0x154
[00144a6c] run_workqueue+0x174/0x220
[00144d26] worker_thread+0x116/0x164
[0014ab9c] kthread+0x158/0x160
[00107c3a] kernel_thread_starter+0x6/0xc
[00107c34] kernel_thread_starter+0x0/0xc
other info that might help us debug this:
1 lock held by kslowcrw/64:
#0: (sd-s_active){}, at: [00209578] remove_dir+0xec/0x144
stack backtrace:
0002 0002 1fd1ba50
1fd1b9c8 004bebf6 004bebf6 00103854
0001 00602cb0
1fd1ba10 1fd1b9b0 000e
0042a780 0010389e 1fd1b9b0 1fd1ba00
Call Trace:
([001037f4] show_trace+0xf8/0xfc)
[001038ce] show_stack+0xd6/0x100
[00103926] dump_stack+0x2e/0x3c
[00154c6e] print_circular_bug_tail+0x9e/0xb0
[0015671c] __lock_acquire+0xd8c/0x1124
[00156e9c] lock_acquire+0x78/0xa8
[00423070] __mutex_lock_slowpath+0xbc/0x3c0
[004233b2] mutex_lock+0x3e/0x4c
[002f0bb6] css_sch_device_unregister+0x32/0x54
[002f174a] css_evaluate_subchannel+0x266/0x3b8
[002f1976] css_trigger_slow_path+0xda/0x154
[00144a6c] run_workqueue+0x174/0x220
[00144d26] worker_thread+0x116/0x164
[0014ab9c] kthread+0x158/0x160
[00107c3a] kernel_thread_starter+0x6/0xc
[00107c34] kernel_thread_starter+0x0/0xc
INFO: lockdep is turned off.
DEV: Unregistering device. ID = '0.0.0010'
bus css: remove device 0.0.0010
DEV:
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
Hello,
Cornelia Huck wrote:
===
[ INFO: possible circular locking dependency detected ]
2.6.21-rc6-ge666c753-dirty #54
---
kslowcrw/64 is trying to acquire lock:
(sch-reg_mutex){--..}, at: [004233b2] mutex_lock+0x3e/0x4c
but task is already holding lock:
(sd-s_active){}, at: [00209578] remove_dir+0xec/0x144
which lock already depends on the new lock.
This is probably because s_active has different write locked and
unlocked by different threads but it doesn't tell lockdep about it.
I'll read lockdep docs and update it.
Thanks for testing.
--
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
Tejun Heo wrote:
Hello,
Cornelia Huck wrote:
===
[ INFO: possible circular locking dependency detected ]
2.6.21-rc6-ge666c753-dirty #54
---
kslowcrw/64 is trying to acquire lock:
(sch-reg_mutex){--..}, at: [004233b2] mutex_lock+0x3e/0x4c
but task is already holding lock:
(sd-s_active){}, at: [00209578] remove_dir+0xec/0x144
which lock already depends on the new lock.
This is probably because s_active has different write locked and
unlocked by different threads but it doesn't tell lockdep about it.
s/has different/is/
Sorry.
--
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
On Wed, Apr 11, 2007 at 06:26:42PM +0900, Tejun Heo wrote:
Hello,
Cornelia Huck wrote:
===
[ INFO: possible circular locking dependency detected ]
2.6.21-rc6-ge666c753-dirty #54
---
kslowcrw/64 is trying to acquire lock:
(sch-reg_mutex){--..}, at: [004233b2] mutex_lock+0x3e/0x4c
but task is already holding lock:
(sd-s_active){}, at: [00209578] remove_dir+0xec/0x144
which lock already depends on the new lock.
This is probably because s_active has different write locked and
unlocked by different threads but it doesn't tell lockdep about it.
I'll read lockdep docs and update it.
Does this patch fix the problem?
Index: work/fs/sysfs/dir.c
===
--- work.orig/fs/sysfs/dir.c2007-04-11 19:12:02.0 +0900
+++ work/fs/sysfs/dir.c 2007-04-11 19:12:12.0 +0900
@@ -26,7 +26,8 @@ void release_sysfs_dirent(struct sysfs_d
* locked. If @sd is cursor for directory walk or being
* released prematurely, s_active has no reader or writer.
*/
- down_write_trylock(sd-s_active);
+ if (!down_write_trylock(sd-s_active))
+ rwsem_acquire(sd-s_active.dep_map, 0, 0, _RET_IP_);
up_write(sd-s_active);
if (sd-s_type SYSFS_KOBJ_LINK)
Index: work/fs/sysfs/sysfs.h
===
--- work.orig/fs/sysfs/sysfs.h 2007-04-11 19:12:02.0 +0900
+++ work/fs/sysfs/sysfs.h 2007-04-11 19:12:03.0 +0900
@@ -171,6 +171,7 @@ static inline void sysfs_put_active_two(
static inline void sysfs_deactivate(struct sysfs_dirent *sd)
{
down_write(sd-s_active);
+ rwsem_release(sd-s_active.dep_map, 1, _RET_IP_);
}
static inline int sysfs_is_shadowed_inode(struct inode *inode)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
On Wed, 11 Apr 2007 19:13:59 +0900,
Tejun Heo [EMAIL PROTECTED] wrote:
Does this patch fix the problem?
Index: work/fs/sysfs/dir.c
===
--- work.orig/fs/sysfs/dir.c 2007-04-11 19:12:02.0 +0900
+++ work/fs/sysfs/dir.c 2007-04-11 19:12:12.0 +0900
@@ -26,7 +26,8 @@ void release_sysfs_dirent(struct sysfs_d
* locked. If @sd is cursor for directory walk or being
* released prematurely, s_active has no reader or writer.
*/
- down_write_trylock(sd-s_active);
+ if (!down_write_trylock(sd-s_active))
+ rwsem_acquire(sd-s_active.dep_map, 0, 0, _RET_IP_);
up_write(sd-s_active);
if (sd-s_type SYSFS_KOBJ_LINK)
Index: work/fs/sysfs/sysfs.h
===
--- work.orig/fs/sysfs/sysfs.h2007-04-11 19:12:02.0 +0900
+++ work/fs/sysfs/sysfs.h 2007-04-11 19:12:03.0 +0900
@@ -171,6 +171,7 @@ static inline void sysfs_put_active_two(
static inline void sysfs_deactivate(struct sysfs_dirent *sd)
{
down_write(sd-s_active);
+ rwsem_release(sd-s_active.dep_map, 1, _RET_IP_);
}
static inline int sysfs_is_shadowed_inode(struct inode *inode)
This seems to work fine now, thanks.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
[PATCH] sysfs: implement sysfs_dirent active reference and immediate disconnect
Opening a sysfs node references its associated kobject, so userland
can arbitrarily prolong lifetime of a kobject which complicates
lifetime rules in drivers. This patch implements active reference and
makes the association between kobject and sysfs immediately breakable.
Now each sysfs_dirent has two reference counts - s_count and s_active.
s_count is a regular reference count which guarantees that the
containing sysfs_dirent is accessible. As long as s_count reference
is held, all sysfs internal fields in sysfs_dirent are accessible
including s_parent and s_name.
The newly added s_active is active reference count. This is acquired
by invoking sysfs_get_active() and it's the caller's responsibility to
ensure sysfs_dirent itself is accessible (should be holding s_count
one way or the other). Dereferencing sysfs_dirent to access objects
out of sysfs proper requires active reference. This includes access
to the associated kobjects, attributes and ops.
The active references can be drained and denied by calling
sysfs_deactivate(). All active sysfs_dirents must be deactivated
after deletion but before the default reference is dropped. This
enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is
deleted, it won't access any entity external to sysfs proper.
Because attr/bin_attr ops access both the node itself and its parent
for kobject, they need to hold active references to both.
sysfs_get/put_active_two() helpers are provided to help grabbing both
references. Parent's is acquired first and released last.
Unlike other operations, mmapped area lingers on after mmap() is
finished and the module implement implementing it and kobj need to
stay referenced till all the mapped pages are gone. This is
accomplished by holding one set of active references to the bin_attr
and its parent if there have been any mmap during lifetime of an
openfile. The references are dropped when the openfile is released.
This change makes sysfs lifetime rules independent from both kobject's
and module's. It not only fixes several race conditions caused by
sysfs not holding onto the proper module when referencing kobject, but
also helps fixing and simplifying lifetime management in driver model
and drivers by taking sysfs out of the equation.
Please read the following message for more info.
http://article.gmane.org/gmane.linux.kernel/510293
Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
Cornelia, this should fix the problem you reported. It's caused by
doing up_write() on sysfs_dirent which is used for readdir cursor
which is not deactivated before being released.
release_sysfs_dirent() is udpated such that it does
down_write_trylock() on s_active before doing up_write(). This also
covers error paths where allocated sysfs_dirent is put due to errors
during initialization.
Andrew, please use this patch instead of the original one.
Thanks.
fs/sysfs/bin.c | 95 +--
fs/sysfs/dir.c | 22 +++--
fs/sysfs/file.c | 132 ---
fs/sysfs/inode.c |8 ++-
fs/sysfs/sysfs.h | 107 +++-
5 files changed, 250 insertions(+), 114 deletions(-)
Index: work/fs/sysfs/dir.c
===
--- work.orig/fs/sysfs/dir.c
+++ work/fs/sysfs/dir.c
@@ -22,6 +22,13 @@ void release_sysfs_dirent(struct sysfs_d
repeat:
parent_sd = sd->s_parent;
+ /* If @sd is being released after deletion, s_active is write
+* locked. If @sd is cursor for directory walk or being
+* released prematurely, s_active has no reader or writer.
+*/
+ down_write_trylock(>s_active);
+ up_write(>s_active);
+
if (sd->s_type & SYSFS_KOBJ_LINK)
sysfs_put(sd->s_elem.symlink.target_sd);
if (sd->s_type & SYSFS_COPY_NAME)
@@ -69,6 +76,7 @@ struct sysfs_dirent *sysfs_new_dirent(co
atomic_set(>s_count, 1);
atomic_set(>s_event, 1);
+ init_rwsem(>s_active);
INIT_LIST_HEAD(>s_children);
INIT_LIST_HEAD(>s_sibling);
@@ -316,7 +324,6 @@ static void remove_dir(struct dentry * d
d_delete(d);
sd = d->d_fsdata;
list_del_init(>s_sibling);
- sysfs_put(sd);
if (d->d_inode)
simple_rmdir(parent->d_inode,d);
@@ -325,6 +332,9 @@ static void remove_dir(struct dentry * d
mutex_unlock(>d_inode->i_mutex);
dput(parent);
+
+ sysfs_deactivate(sd);
+ sysfs_put(sd);
}
void sysfs_remove_subdir(struct dentry * d)
@@ -335,6 +345,7 @@ void sysfs_remove_subdir(struct dentry *
static void __sysfs_remove_dir(struct dentry *dentry)
{
+ LIST_HEAD(removed);
struct sysfs_dirent * parent_sd;
struct sysfs_dirent * sd, * tmp;
@@ -348,12 +359,17 @@ static void __sysfs_remove_dir(struct de
[PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
[PATCH] sysfs: implement sysfs_dirent active reference and immediate disconnect
Opening a sysfs node references its associated kobject, so userland
can arbitrarily prolong lifetime of a kobject which complicates
lifetime rules in drivers. This patch implements active reference and
makes the association between kobject and sysfs immediately breakable.
Now each sysfs_dirent has two reference counts - s_count and s_active.
s_count is a regular reference count which guarantees that the
containing sysfs_dirent is accessible. As long as s_count reference
is held, all sysfs internal fields in sysfs_dirent are accessible
including s_parent and s_name.
The newly added s_active is active reference count. This is acquired
by invoking sysfs_get_active() and it's the caller's responsibility to
ensure sysfs_dirent itself is accessible (should be holding s_count
one way or the other). Dereferencing sysfs_dirent to access objects
out of sysfs proper requires active reference. This includes access
to the associated kobjects, attributes and ops.
The active references can be drained and denied by calling
sysfs_deactivate(). All active sysfs_dirents must be deactivated
after deletion but before the default reference is dropped. This
enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is
deleted, it won't access any entity external to sysfs proper.
Because attr/bin_attr ops access both the node itself and its parent
for kobject, they need to hold active references to both.
sysfs_get/put_active_two() helpers are provided to help grabbing both
references. Parent's is acquired first and released last.
Unlike other operations, mmapped area lingers on after mmap() is
finished and the module implement implementing it and kobj need to
stay referenced till all the mapped pages are gone. This is
accomplished by holding one set of active references to the bin_attr
and its parent if there have been any mmap during lifetime of an
openfile. The references are dropped when the openfile is released.
This change makes sysfs lifetime rules independent from both kobject's
and module's. It not only fixes several race conditions caused by
sysfs not holding onto the proper module when referencing kobject, but
also helps fixing and simplifying lifetime management in driver model
and drivers by taking sysfs out of the equation.
Please read the following message for more info.
http://article.gmane.org/gmane.linux.kernel/510293
Signed-off-by: Tejun Heo [EMAIL PROTECTED]
---
Cornelia, this should fix the problem you reported. It's caused by
doing up_write() on sysfs_dirent which is used for readdir cursor
which is not deactivated before being released.
release_sysfs_dirent() is udpated such that it does
down_write_trylock() on s_active before doing up_write(). This also
covers error paths where allocated sysfs_dirent is put due to errors
during initialization.
Andrew, please use this patch instead of the original one.
Thanks.
fs/sysfs/bin.c | 95 +--
fs/sysfs/dir.c | 22 +++--
fs/sysfs/file.c | 132 ---
fs/sysfs/inode.c |8 ++-
fs/sysfs/sysfs.h | 107 +++-
5 files changed, 250 insertions(+), 114 deletions(-)
Index: work/fs/sysfs/dir.c
===
--- work.orig/fs/sysfs/dir.c
+++ work/fs/sysfs/dir.c
@@ -22,6 +22,13 @@ void release_sysfs_dirent(struct sysfs_d
repeat:
parent_sd = sd-s_parent;
+ /* If @sd is being released after deletion, s_active is write
+* locked. If @sd is cursor for directory walk or being
+* released prematurely, s_active has no reader or writer.
+*/
+ down_write_trylock(sd-s_active);
+ up_write(sd-s_active);
+
if (sd-s_type SYSFS_KOBJ_LINK)
sysfs_put(sd-s_elem.symlink.target_sd);
if (sd-s_type SYSFS_COPY_NAME)
@@ -69,6 +76,7 @@ struct sysfs_dirent *sysfs_new_dirent(co
atomic_set(sd-s_count, 1);
atomic_set(sd-s_event, 1);
+ init_rwsem(sd-s_active);
INIT_LIST_HEAD(sd-s_children);
INIT_LIST_HEAD(sd-s_sibling);
@@ -316,7 +324,6 @@ static void remove_dir(struct dentry * d
d_delete(d);
sd = d-d_fsdata;
list_del_init(sd-s_sibling);
- sysfs_put(sd);
if (d-d_inode)
simple_rmdir(parent-d_inode,d);
@@ -325,6 +332,9 @@ static void remove_dir(struct dentry * d
mutex_unlock(parent-d_inode-i_mutex);
dput(parent);
+
+ sysfs_deactivate(sd);
+ sysfs_put(sd);
}
void sysfs_remove_subdir(struct dentry * d)
@@ -335,6 +345,7 @@ void sysfs_remove_subdir(struct dentry *
static void __sysfs_remove_dir(struct dentry *dentry)
{
+ LIST_HEAD(removed);
struct sysfs_dirent * parent_sd;
struct sysfs_dirent * sd, * tmp;
@@ -348,12 +359,17 @@ static void __sysfs_remove_dir(struct
