Messages in this thread | | | Date | Mon, 4 Jan 2010 10:57:55 -0800 | From | Dmitry Torokhov <> | Subject | Re: drm_vm.c:drm_mmap: possible circular locking dependency detected |
| |
On Sun, Jan 03, 2010 at 02:57:15AM -0800, Eric W. Biederman wrote: > Dmitry Torokhov <dmitry.torokhov@gmail.com> writes: > > > > > Overall I am not concerned about lockdep bitching about serio because it > > still bitches if you simply reload psmouse on a box with Synaptics with a > > pass-through port even though there are nested annotations and it is > > silent first time around. > > This is a new lockdep annotation, and looking into it this appears to > be a true possible deadlock in the serio/sysfs interactions. > > We have serio_pin_driver() called from all of the sysfs attributes > which does: > mutex_lock(&serio->drv_mutex); > > We have serio_disconnect_driver() called on an unplug which does: > mutex_lock(&serio->drv_mutex); > > The deadlock potential is if someone reads say the psmouse rate > sysfs file while the mouse is being unplugged. There is a race > such that we can have: > > sysfs_read_file() > fill_read_buffer() > sysfs_get_active_two() > psmouse_attr_show_helper() > serio_pin_driver() > serio_disconnect_driver() > mutex_lock(&serio->drv_mutex); > <-----------------> mutex_lock(&serio_drv_mutex); > psmouse_disconnect() > sysfs_remove_group(... psmouse_attr_group); > .... > sysfs_deactivate(); > wait_for_completion(); > > > So it is unlikely but possible to deadlock by accessing a serio > attribute of a serio device that is being removed.
Hmm, I guess I was too quick dismissing lockdep complaints here. Now that sysfs remove waits deadlock indeed is possible. Actually the locks on serio->drv_mutex in attributes were added to make sure we don't access device that was unbound from the driver through stale sysfs attributes.
> > What to do about it is another question.
I think we should simply not take serio->drv_mutex in attributes and use driver-private mutex to serialize "set" methods that may alter device state.
-- Dmitry
| |