Re: Sysfs and suicidal attributes

Tejun Heo
2007-07-11T05:04:58Z

Alan Stern wrote:
>> Please also note that currently dev->sem does not protect against adding
>> children. It says it does in the comment on the definition of the field
>> but it's never acquired during device_add().
>
> That's why we need the rwsem.

Alright, there's our confusion. I thought you were gonna use dev->sem
to protect new device addition && driver binding. We can use the same
rwsem directly for binding protection too but I guess there's no big
difference one way or the other.

>> Yeah, I'm all for centralizing things. I just didn't see what could be
>> simplified in LLDs by freezing sysfs access. Can you point me to any
>> example node which can benefit from such thing?
>
> For USB devices, writes to the /sys/.../bConfigurationValue node will
> call usb_set_configuration(), which registers child devices. We don't
> want these calls to fail because a suspend is underway, since suspends
> are supposed to be transparent to userspace. So they need to block
> until the suspend is over.
>
> Access (either read or write) to any sysfs node which actually touches
> the device hardware will fail while the device is suspended. Again,
> for transparency such accesses should block.

Thanks for enlightening me. Probably what can be done is blocking
regular file sysfs nodes automatically and make it optional (optionally
enable or disable) for bin attrs.

>>> Using a freezable workqueue for the callbacks would also localize the
>>> suspend handling.
>> But the freezable workqueue thing isn't really necessary if any of the
>> discussed solutions is implemented, right?
>
> It isn't necessary if the callbacks aren't needed (i.e., if we give
> methods a way to unregister their attribute directly). But if we do
> keep the callbacks, then for safety's sake their workqueue should be
> freezable.

I see. Let's see how the direct suicide thing works out.

Thanks.

--
tejun