On Wednesday, August 28, 2013 08:24:22 AM Tejun Heo wrote: > Hello, Rafael. > > On Tue, Aug 27, 2013 at 11:45:46PM +0200, Rafael J. Wysocki wrote: > > I've thought about that a bit over the last several hours and I'm still > > thinking that that patch is a bit overkill, because it will trigger the > > restart_syscall() for all cases when device_hotplug_lock is locked, even > > if they can't lead to any deadlocks. The only deadlockish situation is > > when device *removal* is in progress when store_online(), for example, > > is called. > > > > So to address that particular situation without adding too much overhead for > > other cases, I've come up with the appended patch (untested for now). > > > > This is how it is supposed to work. > > > > There are three "lock levels" for device hotplug, "normal", "remove" > > and "weak". The difference is related to how __lock_device_hotplug() > > works. Namely, if device hotplug is currently locked, that function > > will either block or return false, depending on the "current lock > > level" and its argument (the "new lock level"). The rules here are > > that false is returned immediately if the "current lock level" is > > "remove" and the "new lock level" is "weak". The function blocks > > for all other combinations of the two. > > I think this is going way too far and a massive overkill in terms of > complexity, which is way more important than for doing some > restart_syscall loops during some rare sysfs file access, which isn't > gonna be that common anyway. Fancy locking always sucks. The root > cause of the problem is file removal ref draining from sysfs side and > a proper solution should be implemented there. Adding all this > complexity to device hotplug lock won't solve problems involving other > locks while obfuscating what's going on through all the complexity. > Also, when sysfs is updated with a proper solution, a simpler > workaround from device hotplug side would be far easier to convert to > the new solution than this, which over time is likely to grow > interesting uses. > > Let's *please* stick to something simple.
OK, I'll use restart_syscall() approach in the simplest form, then. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
