[PATCH 0/7] convert semaphore to mutex in struct class

[PATCH 0/7] convert semaphore to mutex in struct class

[Dave Young]

Convert semaphore to mutex in struct class.
All the patches in this series should be applyed simultaneously

toc:
---
1-driver-core-struct-class-convert-semaphore-to-mutex.patch
2-i2c-struct-class-convert-semaphore-to-mutex.patch
3-ieee1394-struct-class-convert-semaphore-to-mutex.patch
4-power-struct-class-convert-semaphore-to-mutex.patch
5-rtc-struct-class-convert-semaphore-to-mutex.patch
6-scsi-struct-class-convert-semaphore-to-mutex.patch
7-spi-struct-class-convert-semaphore-to-mutex.patch

Summary diffstat:
---
drivers/base/class.c              |   22 ++++++++++----------
drivers/base/core.c               |   13 +++++-------
drivers/i2c/i2c-core.c            |    9 +++-----
drivers/ieee1394/nodemgr.c        |   40 +++++++++++++++++++-------------------
drivers/power/apm_power.c         |    6 ++---
drivers/power/power_supply_core.c |    8 +++----
drivers/rtc/interface.c           |    4 +--
drivers/scsi/hosts.c              |    4 +--
drivers/spi/spi.c                 |    4 +--
include/linux/device.h            |    3 +-
10 files changed, 56 insertions(+), 57 deletions(-)

One lockdep warning detected as following, thus use mutex_lock_nested with SINGLE_DEPTH_NESTING in class_device_add

Jan  3 10:45:15 darkstar kernel: =============================================
Jan  3 10:45:15 darkstar kernel: [ INFO: possible recursive locking detected ]
Jan  3 10:45:15 darkstar kernel: 2.6.24-rc6-mm1-mutex #1
Jan  3 10:45:15 darkstar kernel: ---------------------------------------------
Jan  3 10:45:15 darkstar kernel: modprobe/2130 is trying to acquire lock:
Jan  3 10:45:15 darkstar kernel:  (&cls->mutex){--..}, at: [<c02d4ee0>] class_device_add+0x140/0x240
Jan  3 10:45:15 darkstar kernel:
Jan  3 10:45:15 darkstar kernel: but task is already holding lock:
Jan  3 10:45:15 darkstar kernel:  (&cls->mutex){--..}, at: [<c02d52c3>] class_interface_register+0x43/0xf0
Jan  3 10:45:15 darkstar kernel:
Jan  3 10:45:15 darkstar kernel: other info that might help us debug this:
Jan  3 10:45:15 darkstar kernel: 1 lock held by modprobe/2130:
Jan  3 10:45:15 darkstar kernel:  #0:  (&cls->mutex){--..}, at: [<c02d52c3>] class_interface_register+0x43/0xf0
Jan  3 10:45:15 darkstar kernel:
Jan  3 10:45:15 darkstar kernel: stack backtrace:
Jan  3 10:45:15 darkstar kernel: Pid: 2130, comm: modprobe Not tainted 2.6.24-rc6-mm1-mutex #1
Jan  3 10:45:15 darkstar kernel:  [<c0104d2a>] show_trace_log_lvl+0x1a/0x30
Jan  3 10:45:15 darkstar kernel:  [<c0104d52>] show_trace+0x12/0x20
Jan  3 10:45:15 darkstar kernel:  [<c0104ecd>] dump_stack+0x6d/0x80
Jan  3 10:45:15 darkstar kernel:  [<c0152867>] print_deadlock_bug+0xc7/0xe0
Jan  3 10:45:15 darkstar kernel:  [<c01528ec>] check_deadlock+0x6c/0x80
Jan  3 10:45:15 darkstar kernel:  [<c0152d5c>] validate_chain+0x14c/0x370
Jan  3 10:45:15 darkstar kernel:  [<c01548b0>] __lock_acquire+0x1c0/0x7e0
Jan  3 10:45:15 darkstar kernel:  [<c0155599>] lock_acquire+0x79/0xb0
Jan  3 10:45:15 darkstar kernel:  [<c04252cc>] mutex_lock_nested+0x8c/0x300
Jan  3 10:45:15 darkstar kernel:  [<c02d4ee0>] class_device_add+0x140/0x240
Jan  3 10:45:15 darkstar kernel:  [<c02d4ff2>] class_device_register+0x12/0x20
Jan  3 10:45:15 darkstar kernel:  [<c02d509a>] class_device_create+0x9a/0xb0
Jan  3 10:45:15 darkstar kernel:  [<f886225c>] sg_add+0x12c/0x200 [sg]
Jan  3 10:45:15 darkstar kernel:  [<c02d5359>] class_interface_register+0xd9/0xf0
Jan  3 10:45:15 darkstar kernel:  [<c031242f>] scsi_register_interface+0xf/0x20
Jan  3 10:45:15 darkstar kernel:  [<f882b082>] init_sg+0x82/0xbc [sg]
Jan  3 10:45:15 darkstar kernel:  [<c015e4fa>] sys_init_module+0xea/0x130
Jan  3 10:45:15 darkstar kernel:  [<c0103f42>] syscall_call+0x7/0xb
Jan  3 10:45:15 darkstar kernel:  =======================

If there's anything missed please help to point out, thanks.

Regards
dave

Re: [PATCH 0/7] convert semaphore to mutex in struct class

[Jarek Poplawski]

On Thu, Jan 03, 2008 at 01:50:20PM +0800, Dave Young wrote:
> Convert semaphore to mutex in struct class.
...
> One lockdep warning detected as following, thus use mutex_lock_nested with SINGLE_DEPTH_NESTING in class_device_add
> 
> Jan  3 10:45:15 darkstar kernel: =============================================
> Jan  3 10:45:15 darkstar kernel: [ INFO: possible recursive locking detected ]
> Jan  3 10:45:15 darkstar kernel: 2.6.24-rc6-mm1-mutex #1
> Jan  3 10:45:15 darkstar kernel: ---------------------------------------------
...
> If there's anything missed please help to point out, thanks.

Dave, IMHO it's not 'the right' way to do it: from this and earlier
discussions it seems there could be many more warnings like this one;
lockdep simply always turns itself off after first one. So, merging
your patches like this would effectively turn off lockdep for all
other places as well, maybe for a long time.

I'd suggest to try first to do it with some wrappers around mutexes,
which simply omit lockdep verification, and later try to replace them
one by one, after checking and testing there are no such warnings
anymore (which would often need some additional annotations about
nesting and probably some changes in lockdep too).

Thanks,
Jarek P.

Re: [PATCH 0/7] convert semaphore to mutex in struct class

[Dave Young]

On Jan 3, 2008 3:24 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> On Thu, Jan 03, 2008 at 08:06:09AM +0100, Jarek Poplawski wrote:
> > On Thu, Jan 03, 2008 at 01:50:20PM +0800, Dave Young wrote:
> > > Convert semaphore to mutex in struct class.
> > ...
> > > One lockdep warning detected as following, thus use mutex_lock_nested with SINGLE_DEPTH_NESTING in class_device_add
> > >
> > > Jan  3 10:45:15 darkstar kernel: =============================================
> > > Jan  3 10:45:15 darkstar kernel: [ INFO: possible recursive locking detected ]
> > > Jan  3 10:45:15 darkstar kernel: 2.6.24-rc6-mm1-mutex #1
> > > Jan  3 10:45:15 darkstar kernel: ---------------------------------------------
> > ...
> > > If there's anything missed please help to point out, thanks.
> >
> > Dave, IMHO it's not 'the right' way to do it: [...]
>
> OOPS! (I was sleeping...) Unless it has turned out it's not so hard
> here, and you are quite sure there should be no more warnings after
> this one nesting annotation - then of course, this is the right way!

Thanks ;)
I don't know if there's other possible warning places with this mutex
or not,  if you have any ideas about this, please tell me.

>
> Sorry (?)
> Jarek P.
>

Re: [PATCH 0/7] convert semaphore to mutex in struct class

[Jarek Poplawski]

On Thu, Jan 03, 2008 at 08:06:09AM +0100, Jarek Poplawski wrote:
> On Thu, Jan 03, 2008 at 01:50:20PM +0800, Dave Young wrote:
> > Convert semaphore to mutex in struct class.
> ...
> > One lockdep warning detected as following, thus use mutex_lock_nested with SINGLE_DEPTH_NESTING in class_device_add
> > 
> > Jan  3 10:45:15 darkstar kernel: =============================================
> > Jan  3 10:45:15 darkstar kernel: [ INFO: possible recursive locking detected ]
> > Jan  3 10:45:15 darkstar kernel: 2.6.24-rc6-mm1-mutex #1
> > Jan  3 10:45:15 darkstar kernel: ---------------------------------------------
> ...
> > If there's anything missed please help to point out, thanks.
> 
> Dave, IMHO it's not 'the right' way to do it: [...]

OOPS! (I was sleeping...) Unless it has turned out it's not so hard
here, and you are quite sure there should be no more warnings after
this one nesting annotation - then of course, this is the right way!

Sorry (?)
Jarek P.

Re: [PATCH 0/7] convert semaphore to mutex in struct class

[Jarek Poplawski]

On Thu, Jan 03, 2008 at 03:21:36PM +0800, Dave Young wrote:
...
> I don't know if there's other possible warning places with this mutex
> or not,  if you have any ideas about this, please tell me.

I think lockdep is just to tell such things. So, the question is, how
much it was tested already, because if there are many warnings
reported e.g. after merging to -mm, then this could be better to re-do
it this other way... But, I hope this will not be necessary.

Jarek P.

Re: [PATCH 0/7] convert semaphore to mutex in struct class

[Stefan Richter]

Dave Young wrote:
> Convert semaphore to mutex in struct class.
> All the patches in this series should be applyed simultaneously

Therefore you eventually need to repost it as a single patch.  It can't
go into one of the maintainer trees otherwise, because they only take
fully bijectable patches.  (Kernel must build and run at any point in
between any patch series.)

> toc:
> ---
> 1-driver-core-struct-class-convert-semaphore-to-mutex.patch
> 2-i2c-struct-class-convert-semaphore-to-mutex.patch
> 3-ieee1394-struct-class-convert-semaphore-to-mutex.patch
> 4-power-struct-class-convert-semaphore-to-mutex.patch
> 5-rtc-struct-class-convert-semaphore-to-mutex.patch
> 6-scsi-struct-class-convert-semaphore-to-mutex.patch
> 7-spi-struct-class-convert-semaphore-to-mutex.patch

I was going to test it at runtime on top of 2.6.24-rc6, but the first
and second patch depend on stuff in -mm.  So, my laziness wins for now,
as -mm is not my cup of tea.

About your changelog:

	"xyz: convert semaphore to mutex in struct class

	Use mutex instead of semaphore in struct class.

	Signed-off-by: Dave Young <hidave.darkstar@gmail.com>"

You don't need the second line because it says the same as the first
line.  Either kill it, or replace it by an explanation _why_ the
semaphore is to be replaced by mutex.  (I guess you do it because they
are lighter-weight, both in semantics and in implementation, and because
there are better facilities to debug mutexes...)
-- 
Stefan Richter
-=====-==--- ---= --==-
http://arcgraph.de/sr/