Messages in this thread | | | Date | Fri, 18 Jan 2008 15:48:02 +0800 | From | "Dave Young" <> | Subject | Re: [PATCH 7/7] driver-core : convert semaphore to mutex in struct class |
| |
On Jan 18, 2008 3:38 PM, Jarek Poplawski <jarkao2@gmail.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@vrfy.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. >
| |