linux-kernel - Re: [PATCH 7/7] driver-core : convert semaphore to mutex in struct class

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 18 Jan 2008 15:48:02 +0800
From:	"Dave Young" <hidave.darkstar@...il.com>
To:	"Jarek Poplawski" <jarkao2@...il.com>
Cc:	"Kay Sievers" <kay.sievers@...y.org>,
	"Alan Stern" <stern@...land.harvard.edu>,
	"Greg KH" <gregkh@...e.de>, stefanr@...6.in-berlin.de,
	"David Brownell" <david-b@...bell.net>,
	"Kernel development list" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 7/7] driver-core : convert semaphore to mutex in struct class

On Jan 18, 2008 3:38 PM, Jarek Poplawski <jarkao2@...il.com> wrote:
> On Fri, Jan 18, 2008 at 01:31:17PM +0800, Dave Young wrote:
> > On Jan 18, 2008 11:18 AM, Kay Sievers <kay.sievers@...y.org> wrote:
> ...
> > > Yeah, might be better to wait until class_device is gone, otherwise you
> > > may need to fix stuff that is just going to be removed. Your change to
> > > have iterators for the class devices look like a nice preparation for
> > > future changes though.
> > >
> > > Our rough plan is:
> > >  2.6.25:
> > >   - get the ~100 patches in Greg's tree (in -mm) merged :)
> > >  2.6.26:
> > > ???  - remove the 20 char limit in "struct device"
> > >   - get rid of "struct class_device"
> >
> > Fine, thanks.
> >
> > Let's wait for other people's comment.
>
> Dave, I doubt you'll ever manage to do this if you're going to wait:
> probably there will be always some new changes like this around...

Maybe you are right.

>
> IMHO, it would be nice to get the real state of current lockdep
> problems here to figure out if there is any chance to do this right &
> without any warnings with current lockdep. If I got it right from
> earlier threads it might be impossible with USB, at least.

I don't think so, usb doesn't be affected by struct class mutex, they
only use the lock of struct device. As I replied before, the lockdep
issue exist only between class_interface and class_device.

>
> So, since I think these nesting levels seem to be wrong in 7/7 patch,
> maybe it's better to exclude it from this patchset, and to try this as
> testing for some time.

I may file the updated patch with more nesting changes and test it of
course. Actually I should have done it, thanks.

>
> My proposal is to do the annotations with mutex_lock_nested(),
> everywhere in this patch, according to 'real' relations between these
> classes from thread POV, so e.g.:
>
> mutex_lock_nested(&some_class->mutex, CLASS_PARENT);
> mutex_lock_nested(&some_class->mutex, CLASS_CLASS);
> ...or more if needed:
> mutex_lock_nested(&some_class->mutex, CLASS_CHILD);
> mutex_lock_nested(&some_class->mutex, CLASS_ROOT);
> ...etc.
>
> using adequate names for these enums, and only after this check what
> lockdep thinks about it. Then, if there are no obvious mistakes, but
> lockdep doesn't like it, send the patch and report without trying to
> 'silence' lockdep, so we could see what's going on, and if there are
> any chances to make it right.

1) Using CLASS_NORMAL/CLASS_PARENT/CLASS_CHILD will be enough.
or
2) Simply add SINGLE_LEVEL_NESTING in class_device_add and other
class_device functions because it is the only possible nest-lock place
as I know.

Do you agree?

>
> Thanks,
> Jarek P.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/