Re: drm_vm.c:drm_mmap: possible circular locking dependency detected (was: Re: Linux 2.6.33-rc2 - Merry Christmas ...)

[ebiederm@xmission.com (Eric W. Biederman)]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Thu, 31 Dec 2009, Eric W. Biederman wrote:
>> >
>> >  - hibernate ends up with the sequence: _cpu_down (cpu_hotplug.lock) ->  ..
>> >    kref_put .. -> sysfs_addrm_start (sysfs_mutex)
>> >
>> >    Again, nothing suspicious or "bad", and this part of the dependency 
>> >    chain has nothing to do with the DRM code itself.
>> 
>> kobject_del with a lock held scares me.
>
> I would not object at _all_ if sysfs fixed the locking here instead of in 
> filldir.

I just sent you my sysfs filldir scalability patch, so we can take that
red-herring off the plate.

The problem as I see it is that kobject_del is convenient.
kobject_del waits until all of the sysfs show and store methods for
that kobject have stopped executing.  Which imposes the rule that
kobject_del can not be called with any locks held that are taken in a
sysfs show or store method.  This is all invisible to lockdep as the
wait is done with a completion and not a lock.

Which unfortunately means fixing filldir only removes some noise from
the picture, and completely hides the problem from lockdep.


....

Looking at the case I am familiar with in the networking layer I think
I have stumbled on a way to sort out this locking problem.

Today the network layer effectively does:
rtnl_lock();
device_del(dev);
rtnl_unlock();
kobject_put(dev);


sysfs_deactivate happens in the device_del(), but if we were to move
sysfs_deactivate into the final kobject_put then in theory we can
continue to block and be friendly but not need to be called with
locations where locks are held.

The core idea is to allow unlisting devices from sysfs under a lock
while still waiting for all users to complete after it is safe to
drop the lock.

Does that work for the cpu hotplug case?  Doing everything from
notifiers makes me suspect it will fail.


....


Either way we will need some lockdep warnings for sysfs_deactivate
so that the problem does not continue to hide and silently foul
things up.  So I will see if I can cook something.

Eric