Hi,
There is currently no generic trigger for userspace to know when a driver
is bound to a device. Such a trigger may be required in cases where setup
steps must be performed in userspace after the device is bound, e.g.
because the driver adds sysfs attributes in its probe function.
I can imagine 3 possible ways to solve this problem:
* add a bus specific change event (triggered by BUS_NOTIFY_BOUND_DRIVER)
- this may result in duplicated code for each bus
* dissable autoprobing and "manually" probe the device from userspace
triggered by the add event - this duplicates logic already implemented
in the kernel
* add a generic bind/unbind uevent
Which one is preferred from a driver core perspective?
Regards,
Sebastian
On Tue, Dec 07, 2010 at 05:18:27PM +0100, Sebastian Ott wrote:
> Hi,
>
> There is currently no generic trigger for userspace to know when a driver
> is bound to a device.
Not true at all, you get one when a device is attached to a bus. What's
wrong with that notification?
> Such a trigger may be required in cases where setup
> steps must be performed in userspace after the device is bound, e.g.
> because the driver adds sysfs attributes in its probe function.
A driver should not add sysfs attributes in its probe function as that
is racy as you have noticed. Add the attributes in the bus functions
for that driver and it should be fine.
> I can imagine 3 possible ways to solve this problem:
> * add a bus specific change event (triggered by BUS_NOTIFY_BOUND_DRIVER)
> - this may result in duplicated code for each bus
> * dissable autoprobing and "manually" probe the device from userspace
> triggered by the add event - this duplicates logic already implemented
> in the kernel
> * add a generic bind/unbind uevent
>
> Which one is preferred from a driver core perspective?
None, use the existing notifications like everyone else :)
thanks,
greg k-h
On Tue, 7 Dec 2010, Greg KH wrote:
> On Tue, Dec 07, 2010 at 05:18:27PM +0100, Sebastian Ott wrote:
> > Hi,
> >
> > There is currently no generic trigger for userspace to know when a driver
> > is bound to a device.
>
> Not true at all, you get one when a device is attached to a bus. What's
> wrong with that notification?
we get a KOBJ_ADD if a device is attached to a bus, but this does not
imply that a device driver is bound to this device
>
> > Such a trigger may be required in cases where setup
> > steps must be performed in userspace after the device is bound, e.g.
> > because the driver adds sysfs attributes in its probe function.
>
> A driver should not add sysfs attributes in its probe function as that
> is racy as you have noticed. Add the attributes in the bus functions
> for that driver and it should be fine.
sry..I was not clear on this one. I was talking driver specific
attributes per device. So I'm searching for a trigger when these
attributes are created, or in other words when the device is useable,
which I think translates to when a driver is bound to this device.
>
> > I can imagine 3 possible ways to solve this problem:
> > * add a bus specific change event (triggered by BUS_NOTIFY_BOUND_DRIVER)
> > - this may result in duplicated code for each bus
> > * dissable autoprobing and "manually" probe the device from userspace
> > triggered by the add event - this duplicates logic already implemented
> > in the kernel
> > * add a generic bind/unbind uevent
> >
> > Which one is preferred from a driver core perspective?
>
> None, use the existing notifications like everyone else :)
>
> thanks,
>
> greg k-h
>
On Tue, Dec 07, 2010 at 06:29:37PM +0100, Sebastian Ott wrote:
>
> On Tue, 7 Dec 2010, Greg KH wrote:
> > On Tue, Dec 07, 2010 at 05:18:27PM +0100, Sebastian Ott wrote:
> > > Hi,
> > >
> > > There is currently no generic trigger for userspace to know when a driver
> > > is bound to a device.
> >
> > Not true at all, you get one when a device is attached to a bus. What's
> > wrong with that notification?
> we get a KOBJ_ADD if a device is attached to a bus, but this does not
> imply that a device driver is bound to this device
You can get that information from that uevent, it's all there for you to
listen to.
> > > Such a trigger may be required in cases where setup
> > > steps must be performed in userspace after the device is bound, e.g.
> > > because the driver adds sysfs attributes in its probe function.
> >
> > A driver should not add sysfs attributes in its probe function as that
> > is racy as you have noticed. Add the attributes in the bus functions
> > for that driver and it should be fine.
> sry..I was not clear on this one. I was talking driver specific
> attributes per device.
No, I understand.
> So I'm searching for a trigger when these attributes are created, or
> in other words when the device is useable, which I think translates to
> when a driver is bound to this device.
Again, KOBJ_ADD is the correct one.
If your driver is creating sysfs attributes on its own, that's a bug and
should be fixed.
thanks,
greg k-h
On Tue, Dec 7, 2010 at 19:33, Greg KH <[email protected]> wrote:
> On Tue, Dec 07, 2010 at 06:29:37PM +0100, Sebastian Ott wrote:
>> So I'm searching for a trigger when these attributes are created, or
>> in other words when the device is useable, which I think translates to
>> when a driver is bound to this device.
>
> Again, KOBJ_ADD is the correct one.
>
> If your driver is creating sysfs attributes on its own, that's a bug and
> should be fixed.
Sounds a bit like the driver should create its own child device with
its own properties, instead of mangling around with attributes at a
device it binds to.
Sebastian, care to be more specific about the problem and bus in
question. We should avoid new generic events. In some special cases,
sending out 'change' from the driver might be acceptable, but it's
usually not what we would suggest.
Kay
On Tue, 7 Dec 2010, Greg KH wrote:
> On Tue, Dec 07, 2010 at 06:29:37PM +0100, Sebastian Ott wrote:
> >
> > On Tue, 7 Dec 2010, Greg KH wrote:
> > > On Tue, Dec 07, 2010 at 05:18:27PM +0100, Sebastian Ott wrote:
> > > > Hi,
> > > >
> > > > There is currently no generic trigger for userspace to know when a driver
> > > > is bound to a device.
> > >
> > > Not true at all, you get one when a device is attached to a bus. What's
> > > wrong with that notification?
> > we get a KOBJ_ADD if a device is attached to a bus, but this does not
> > imply that a device driver is bound to this device
>
> You can get that information from that uevent, it's all there for you to
> listen to.
>
> > > > Such a trigger may be required in cases where setup
> > > > steps must be performed in userspace after the device is bound, e.g.
> > > > because the driver adds sysfs attributes in its probe function.
> > >
> > > A driver should not add sysfs attributes in its probe function as that
> > > is racy as you have noticed. Add the attributes in the bus functions
> > > for that driver and it should be fine.
> > sry..I was not clear on this one. I was talking driver specific
> > attributes per device.
>
> No, I understand.
>
> > So I'm searching for a trigger when these attributes are created, or
> > in other words when the device is useable, which I think translates to
> > when a driver is bound to this device.
>
> Again, KOBJ_ADD is the correct one.
>
> If your driver is creating sysfs attributes on its own, that's a bug and
> should be fixed.
Ok, understood. The sad part is, that virtually all s390 device drivers
do this. I've not checked the archives, but i guess this is the status
since the 2.6 release. I don't think that changing all those drivers (not
to mention userspace relying on the structure) is an option here.
Regards,
Sebastian
On Tue, 7 Dec 2010, Kay Sievers wrote:
> On Tue, Dec 7, 2010 at 19:33, Greg KH <[email protected]> wrote:
> > On Tue, Dec 07, 2010 at 06:29:37PM +0100, Sebastian Ott wrote:
>
> >> So I'm searching for a trigger when these attributes are created, or
> >> in other words when the device is useable, which I think translates to
> >> when a driver is bound to this device.
> >
> > Again, KOBJ_ADD is the correct one.
> >
> > If your driver is creating sysfs attributes on its own, that's a bug and
> > should be fixed.
>
> Sounds a bit like the driver should create its own child device with
> its own properties, instead of mangling around with attributes at a
> device it binds to.
Yes, I get that feeling too. But I'm talking about existing drivers
and I don't think I can change their whole structure.
>
> Sebastian, care to be more specific about the problem and bus in
> question. We should avoid new generic events. In some special cases,
> sending out 'change' from the driver might be acceptable, but it's
> usually not what we would suggest.
The specific problem I'm facing is about network drivers on S390.
They don't have one network adaptor, but deal with multiple devices
(e.g. a read, write and a data channel). To handle this we create
a group device to combine these devices. Triggered from userspace,
a network driver will create a group device and wait for itself to
bind to it (and adds his attributes inside his probe function).
Now some udev scripts expect these attributes at the time they
receive the add event. I don't want each network driver to throw
their own change events. So I guess I should go for the bus specific
change event. An other option would be to delay the add event..
which would be ugly, but has the advantage that we don't need to
change the udev scripts (which are distributor specific).
Regards,
Sebastian
On Wed, Dec 08, 2010 at 11:16:46AM +0100, Sebastian Ott wrote:
> Ok, understood. The sad part is, that virtually all s390 device drivers
> do this.
Then they are broken, please fix them :)
> I've not checked the archives, but i guess this is the status
> since the 2.6 release. I don't think that changing all those drivers (not
> to mention userspace relying on the structure) is an option here.
Not true, there's really not that many s390 drivers, just fix them to do
the correct thing here, no userspace needs to be changed at all.
thanks,
greg k-h
On Wed, Dec 08, 2010 at 11:18:27AM +0100, Sebastian Ott wrote:
> On Tue, 7 Dec 2010, Kay Sievers wrote:
> > On Tue, Dec 7, 2010 at 19:33, Greg KH <[email protected]> wrote:
> > > On Tue, Dec 07, 2010 at 06:29:37PM +0100, Sebastian Ott wrote:
> >
> > >> So I'm searching for a trigger when these attributes are created, or
> > >> in other words when the device is useable, which I think translates to
> > >> when a driver is bound to this device.
> > >
> > > Again, KOBJ_ADD is the correct one.
> > >
> > > If your driver is creating sysfs attributes on its own, that's a bug and
> > > should be fixed.
> >
> > Sounds a bit like the driver should create its own child device with
> > its own properties, instead of mangling around with attributes at a
> > device it binds to.
>
> Yes, I get that feeling too. But I'm talking about existing drivers
> and I don't think I can change their whole structure.
It's just a matter of putting the attributes in a table and passing that
to the bus code the driver registers with. Only a minor change in the
driver is needed to resolve this.
good luck,
greg k-h
On Wed, 8 Dec 2010, Greg KH wrote:
> On Wed, Dec 08, 2010 at 11:18:27AM +0100, Sebastian Ott wrote:
> > On Tue, 7 Dec 2010, Kay Sievers wrote:
> > > On Tue, Dec 7, 2010 at 19:33, Greg KH <[email protected]> wrote:
> > > > On Tue, Dec 07, 2010 at 06:29:37PM +0100, Sebastian Ott wrote:
> > >
> > > >> So I'm searching for a trigger when these attributes are created, or
> > > >> in other words when the device is useable, which I think translates to
> > > >> when a driver is bound to this device.
> > > >
> > > > Again, KOBJ_ADD is the correct one.
> > > >
> > > > If your driver is creating sysfs attributes on its own, that's a bug and
> > > > should be fixed.
> > >
> > > Sounds a bit like the driver should create its own child device with
> > > its own properties, instead of mangling around with attributes at a
> > > device it binds to.
> >
> > Yes, I get that feeling too. But I'm talking about existing drivers
> > and I don't think I can change their whole structure.
>
> It's just a matter of putting the attributes in a table and passing that
> to the bus code the driver registers with. Only a minor change in the
> driver is needed to resolve this.
I don't get it. The current situation is, that our drivers
add attributes to the device they are about to bind with, in
the probe function. I think we agree that it would be better
if those drivers would register a child device and add attributes
to this one. But my concern is that this would break userspace
which walks the sysfs tree and changes attributes, since it changed
from:
/sys/devices/$DEV/$ATTR
to:
/sys/devices/$DEV/$CHILD/$ATTR
>
> good luck,
>
> greg k-h
>
On Mon, Dec 13, 2010 at 08:27:45PM +0100, Sebastian Ott wrote:
> On Wed, 8 Dec 2010, Greg KH wrote:
> > On Wed, Dec 08, 2010 at 11:18:27AM +0100, Sebastian Ott wrote:
> > > On Tue, 7 Dec 2010, Kay Sievers wrote:
> > > > On Tue, Dec 7, 2010 at 19:33, Greg KH <[email protected]> wrote:
> > > > > On Tue, Dec 07, 2010 at 06:29:37PM +0100, Sebastian Ott wrote:
> > > >
> > > > >> So I'm searching for a trigger when these attributes are created, or
> > > > >> in other words when the device is useable, which I think translates to
> > > > >> when a driver is bound to this device.
> > > > >
> > > > > Again, KOBJ_ADD is the correct one.
> > > > >
> > > > > If your driver is creating sysfs attributes on its own, that's a bug and
> > > > > should be fixed.
> > > >
> > > > Sounds a bit like the driver should create its own child device with
> > > > its own properties, instead of mangling around with attributes at a
> > > > device it binds to.
> > >
> > > Yes, I get that feeling too. But I'm talking about existing drivers
> > > and I don't think I can change their whole structure.
> >
> > It's just a matter of putting the attributes in a table and passing that
> > to the bus code the driver registers with. Only a minor change in the
> > driver is needed to resolve this.
>
> I don't get it. The current situation is, that our drivers
> add attributes to the device they are about to bind with, in
> the probe function.
Don't do that, have your _driver_ register the attributes with the bus
it is on, then when the binding happens, the attributes will
automatically get created for the device before the notification is sent
to userspace. That is the proper proceedure here.
> I think we agree that it would be better
> if those drivers would register a child device and add attributes
> to this one. But my concern is that this would break userspace
> which walks the sysfs tree and changes attributes, since it changed
> from:
> /sys/devices/$DEV/$ATTR
> to:
> /sys/devices/$DEV/$CHILD/$ATTR
Ick, no, you don't have to do that, just use the functionality the
driver core provides for you already.
thanks,
greg k-h
On Mon, 13 Dec 2010, Greg KH wrote:
> On Mon, Dec 13, 2010 at 08:27:45PM +0100, Sebastian Ott wrote:
> > On Wed, 8 Dec 2010, Greg KH wrote:
> > > On Wed, Dec 08, 2010 at 11:18:27AM +0100, Sebastian Ott wrote:
> > > > On Tue, 7 Dec 2010, Kay Sievers wrote:
> > > > > On Tue, Dec 7, 2010 at 19:33, Greg KH <[email protected]> wrote:
> > > > > > On Tue, Dec 07, 2010 at 06:29:37PM +0100, Sebastian Ott wrote:
> > > > >
> > > > > >> So I'm searching for a trigger when these attributes are created, or
> > > > > >> in other words when the device is useable, which I think translates to
> > > > > >> when a driver is bound to this device.
> > > > > >
> > > > > > Again, KOBJ_ADD is the correct one.
> > > > > >
> > > > > > If your driver is creating sysfs attributes on its own, that's a bug and
> > > > > > should be fixed.
> > > > >
> > > > > Sounds a bit like the driver should create its own child device with
> > > > > its own properties, instead of mangling around with attributes at a
> > > > > device it binds to.
> > > >
> > > > Yes, I get that feeling too. But I'm talking about existing drivers
> > > > and I don't think I can change their whole structure.
> > >
> > > It's just a matter of putting the attributes in a table and passing that
> > > to the bus code the driver registers with. Only a minor change in the
> > > driver is needed to resolve this.
> >
> > I don't get it. The current situation is, that our drivers
> > add attributes to the device they are about to bind with, in
> > the probe function.
>
> Don't do that, have your _driver_ register the attributes with the bus
> it is on, then when the binding happens, the attributes will
> automatically get created for the device before the notification is sent
> to userspace. That is the proper proceedure here.
are you suggesting that these driver specific device attributes should be
created by the bus code which registers the device?
at this time it is not determinable which driver will bind to the device
(and therefore which attributes to create), also driver specific data may
not be initialized.
>
> > I think we agree that it would be better
> > if those drivers would register a child device and add attributes
> > to this one. But my concern is that this would break userspace
> > which walks the sysfs tree and changes attributes, since it changed
> > from:
> > /sys/devices/$DEV/$ATTR
> > to:
> > /sys/devices/$DEV/$CHILD/$ATTR
>
> Ick, no, you don't have to do that, just use the functionality the
> driver core provides for you already.
>
> thanks,
>
> greg k-h
>
On Tue, Dec 14, 2010 at 07:26:40PM +0100, Sebastian Ott wrote:
>
> On Mon, 13 Dec 2010, Greg KH wrote:
> > On Mon, Dec 13, 2010 at 08:27:45PM +0100, Sebastian Ott wrote:
> > > On Wed, 8 Dec 2010, Greg KH wrote:
> > > > On Wed, Dec 08, 2010 at 11:18:27AM +0100, Sebastian Ott wrote:
> > > > > On Tue, 7 Dec 2010, Kay Sievers wrote:
> > > > > > On Tue, Dec 7, 2010 at 19:33, Greg KH <[email protected]> wrote:
> > > > > > > On Tue, Dec 07, 2010 at 06:29:37PM +0100, Sebastian Ott wrote:
> > > > > >
> > > > > > >> So I'm searching for a trigger when these attributes are created, or
> > > > > > >> in other words when the device is useable, which I think translates to
> > > > > > >> when a driver is bound to this device.
> > > > > > >
> > > > > > > Again, KOBJ_ADD is the correct one.
> > > > > > >
> > > > > > > If your driver is creating sysfs attributes on its own, that's a bug and
> > > > > > > should be fixed.
> > > > > >
> > > > > > Sounds a bit like the driver should create its own child device with
> > > > > > its own properties, instead of mangling around with attributes at a
> > > > > > device it binds to.
> > > > >
> > > > > Yes, I get that feeling too. But I'm talking about existing drivers
> > > > > and I don't think I can change their whole structure.
> > > >
> > > > It's just a matter of putting the attributes in a table and passing that
> > > > to the bus code the driver registers with. Only a minor change in the
> > > > driver is needed to resolve this.
> > >
> > > I don't get it. The current situation is, that our drivers
> > > add attributes to the device they are about to bind with, in
> > > the probe function.
> >
> > Don't do that, have your _driver_ register the attributes with the bus
> > it is on, then when the binding happens, the attributes will
> > automatically get created for the device before the notification is sent
> > to userspace. That is the proper proceedure here.
>
> are you suggesting that these driver specific device attributes should be
> created by the bus code which registers the device?
Yes.
> at this time it is not determinable which driver will bind to the device
> (and therefore which attributes to create), also driver specific data may
> not be initialized.
{sigh}
Please go _look_ at how the driver model handles this type of thing. It
does _exactly_ what you want, as we have been doing it for _years_
properly.
thanks,
greg k-h
On Tue, 14 Dec 2010 11:29:52 -0800,
Greg KH <[email protected]> wrote:
> On Tue, Dec 14, 2010 at 07:26:40PM +0100, Sebastian Ott wrote:
> >
> > On Mon, 13 Dec 2010, Greg KH wrote:
> > > On Mon, Dec 13, 2010 at 08:27:45PM +0100, Sebastian Ott wrote:
> > > Don't do that, have your _driver_ register the attributes with the bus
> > > it is on, then when the binding happens, the attributes will
> > > automatically get created for the device before the notification is sent
> > > to userspace. That is the proper proceedure here.
> >
> > are you suggesting that these driver specific device attributes should be
> > created by the bus code which registers the device?
>
> Yes.
>
> > at this time it is not determinable which driver will bind to the device
> > (and therefore which attributes to create), also driver specific data may
> > not be initialized.
>
> {sigh}
>
> Please go _look_ at how the driver model handles this type of thing. It
> does _exactly_ what you want, as we have been doing it for _years_
> properly.
>
I don't understand how this could work. All of the attribute (groups)
registered before the uevent are driver-ignorant. If the bus wants to
specify the attributes, it needs to know the driver the device will
bind to (regardless of whatever tables exist that show the driver <->
attribute relationship). But it cannot know the driver until after the
uevent. Or else it would need to create _all_ attributes for _all_
devices (surely you didn't mean that)? And what happens with drivers
that are loaded later on?
Could you please elaborate on how the attributes could be created in a
compatible way with today's driver core?
Cornelia
On Wed, Dec 15, 2010 at 02:21:13PM +0100, Cornelia Huck wrote:
> On Tue, 14 Dec 2010 11:29:52 -0800,
> Greg KH <[email protected]> wrote:
>
> > On Tue, Dec 14, 2010 at 07:26:40PM +0100, Sebastian Ott wrote:
> > >
> > > On Mon, 13 Dec 2010, Greg KH wrote:
> > > > On Mon, Dec 13, 2010 at 08:27:45PM +0100, Sebastian Ott wrote:
>
> > > > Don't do that, have your _driver_ register the attributes with the bus
> > > > it is on, then when the binding happens, the attributes will
> > > > automatically get created for the device before the notification is sent
> > > > to userspace. That is the proper proceedure here.
> > >
> > > are you suggesting that these driver specific device attributes should be
> > > created by the bus code which registers the device?
> >
> > Yes.
> >
> > > at this time it is not determinable which driver will bind to the device
> > > (and therefore which attributes to create), also driver specific data may
> > > not be initialized.
> >
> > {sigh}
> >
> > Please go _look_ at how the driver model handles this type of thing. It
> > does _exactly_ what you want, as we have been doing it for _years_
> > properly.
> >
>
> I don't understand how this could work. All of the attribute (groups)
> registered before the uevent are driver-ignorant. If the bus wants to
> specify the attributes, it needs to know the driver the device will
> bind to (regardless of whatever tables exist that show the driver <->
> attribute relationship). But it cannot know the driver until after the
> uevent. Or else it would need to create _all_ attributes for _all_
> devices (surely you didn't mean that)? And what happens with drivers
> that are loaded later on?
>
> Could you please elaborate on how the attributes could be created in a
> compatible way with today's driver core?
Come on people, just look at the code! It does exactly this today just
fine with no changes needed to the driver core.
How about I turn it around for you, please show me how the driver core
does _not_ support this today? If you can prove that this isn't working
properly, then great, I'll gladly accept patches to resolve it.
thanks,
greg k-h
On Wed, 15 Dec 2010 08:23:16 -0800,
Greg KH <[email protected]> wrote:
> On Wed, Dec 15, 2010 at 02:21:13PM +0100, Cornelia Huck wrote:
> > On Tue, 14 Dec 2010 11:29:52 -0800,
> > Greg KH <[email protected]> wrote:
> >
> > > On Tue, Dec 14, 2010 at 07:26:40PM +0100, Sebastian Ott wrote:
> > > >
> > > > On Mon, 13 Dec 2010, Greg KH wrote:
> > > > > On Mon, Dec 13, 2010 at 08:27:45PM +0100, Sebastian Ott wrote:
> >
> > > > > Don't do that, have your _driver_ register the attributes with the bus
> > > > > it is on, then when the binding happens, the attributes will
> > > > > automatically get created for the device before the notification is sent
> > > > > to userspace. That is the proper proceedure here.
> > > >
> > > > are you suggesting that these driver specific device attributes should be
> > > > created by the bus code which registers the device?
> > >
> > > Yes.
> > >
> > > > at this time it is not determinable which driver will bind to the device
> > > > (and therefore which attributes to create), also driver specific data may
> > > > not be initialized.
> > >
> > > {sigh}
> > >
> > > Please go _look_ at how the driver model handles this type of thing. It
> > > does _exactly_ what you want, as we have been doing it for _years_
> > > properly.
> > >
> >
> > I don't understand how this could work. All of the attribute (groups)
> > registered before the uevent are driver-ignorant. If the bus wants to
> > specify the attributes, it needs to know the driver the device will
> > bind to (regardless of whatever tables exist that show the driver <->
> > attribute relationship). But it cannot know the driver until after the
> > uevent. Or else it would need to create _all_ attributes for _all_
> > devices (surely you didn't mean that)? And what happens with drivers
> > that are loaded later on?
> >
> > Could you please elaborate on how the attributes could be created in a
> > compatible way with today's driver core?
>
> Come on people, just look at the code! It does exactly this today just
> fine with no changes needed to the driver core.
>
> How about I turn it around for you, please show me how the driver core
> does _not_ support this today? If you can prove that this isn't working
> properly, then great, I'll gladly accept patches to resolve it.
Looking at device_add():
various setup
add kobject to tree
...
call device_add_attrs
(this adds pre-specified attribute
groups, depending on dev->type,
dev->class or the device specific
attribute groups)
call bus_add_device
(add per-bus pre-specified attributes;
the bus does not know the driver yet)
...
call kobject_uevent <--- first time userspace knows
about device
call bus_probe_device <--- first time a driver may
actually see the device
(depending on the ->match
and ->probe functions,
this may be quite a bit of
time afterwards)
This will not be a problem if a device driver registers a child device
(since it can specify the attributes there).
I think the basic problem is that the KOBJ_ADD uevent notifies
userspace that "a device is there", while the device will only be
really useable by userspace once a driver has bound to it. A module
load triggered by KOBJ_ADD is fine, but trying to actually use the
device after KOBJ_ADD is racy. This will not matter in the usual case,
since either the matching/probing is fast enough or userspace will wait
for something like a block device anyway, but we've seen problems on
s390. A KOBJ_BIND/UNBIND would make a proper distinction between
"device is there" and "device is usable".
(Besides, what happens on unbind/bind? Shouldn't userspace know that a
device is now bound to a different driver?)
Cornelia
2010/12/15 Cornelia Huck <[email protected]>:
> On Wed, 15 Dec 2010 08:23:16 -0800, Greg KH <[email protected]> wrote:
>> On Wed, Dec 15, 2010 at 02:21:13PM +0100, Cornelia Huck wrote:
>> How about I turn it around for you, please show me how the driver core
>> does _not_ support this today? If you can prove that this isn't working
>> properly, then great, I'll gladly accept patches to resolve it.
>
> Looking at device_add():
...
> This will not be a problem if a device driver registers a child device
> (since it can specify the attributes there).
Which is the proper way to do it. No driver should ever mangle a
device which it does not own. It's like adding properties of a block
device directly to a usb_interface device. That just can not work
correctly for many reasons, inside and outside of the kernel.
> I think the basic problem is that the KOBJ_ADD uevent notifies
> userspace that "a device is there", while the device will only be
> really useable by userspace once a driver has bound to it.
This device represents a device on a bus, and can usually do its own
things. A driver can bind to it, but should not mangle it.
> A module
> load triggered by KOBJ_ADD is fine, but trying to actually use the
> device after KOBJ_ADD is racy. This will not matter in the usual case,
> since either the matching/probing is fast enough or userspace will wait
> for something like a block device anyway, but we've seen problems on
> s390. A KOBJ_BIND/UNBIND would make a proper distinction between
> "device is there" and "device is usable".
We don't rally want any such events. We expect a new child device
being created from the driver, instead of re-using the existing bus
device.
> (Besides, what happens on unbind/bind? Shouldn't userspace know that a
> device is now bound to a different driver?)
It does that by watching the child devices the driver creates and destroys.
We already have enough events to handle on today's boxes, we really
don't want to add new ones, which are only needed to work around such
use cases, which ideally just should be fixed.
If you can not change the current drivers to create child devices, the
driver can probably just send change events for the already existing
devices it mangles from the driver.
We don't want to encourage any such use model in general, and such
hacks should be bus/driver specific (and only for legacy reasons), and
they do not belong into the driver core.
Kay
On Wed, 15 Dec 2010 18:51:48 +0100,
Kay Sievers <[email protected]> wrote:
> 2010/12/15 Cornelia Huck <[email protected]>:
> > On Wed, 15 Dec 2010 08:23:16 -0800, Greg KH <[email protected]> wrote:
> >> On Wed, Dec 15, 2010 at 02:21:13PM +0100, Cornelia Huck wrote:
>
> >> How about I turn it around for you, please show me how the driver core
> >> does _not_ support this today? ?If you can prove that this isn't working
> >> properly, then great, I'll gladly accept patches to resolve it.
> >
> > Looking at device_add():
>
> ...
>
> > This will not be a problem if a device driver registers a child device
> > (since it can specify the attributes there).
>
> Which is the proper way to do it. No driver should ever mangle a
> device which it does not own. It's like adding properties of a block
> device directly to a usb_interface device. That just can not work
> correctly for many reasons, inside and outside of the kernel.
That's fine for new device drivers.
>
> > I think the basic problem is that the KOBJ_ADD uevent notifies
> > userspace that "a device is there", while the device will only be
> > really useable by userspace once a driver has bound to it.
>
> This device represents a device on a bus, and can usually do its own
> things. A driver can bind to it, but should not mangle it.
>
> > A module
> > load triggered by KOBJ_ADD is fine, but trying to actually use the
> > device after KOBJ_ADD is racy. This will not matter in the usual case,
> > since either the matching/probing is fast enough or userspace will wait
> > for something like a block device anyway, but we've seen problems on
> > s390. A KOBJ_BIND/UNBIND would make a proper distinction between
> > "device is there" and "device is usable".
>
> We don't rally want any such events. We expect a new child device
> being created from the driver, instead of re-using the existing bus
> device.
Do we want to force a device driver to create a child device just to
notify userspace of the bind?
>
> > (Besides, what happens on unbind/bind? Shouldn't userspace know that a
> > device is now bound to a different driver?)
>
> It does that by watching the child devices the driver creates and destroys.
>
> We already have enough events to handle on today's boxes, we really
> don't want to add new ones, which are only needed to work around such
> use cases, which ideally just should be fixed.
>
> If you can not change the current drivers to create child devices, the
> driver can probably just send change events for the already existing
> devices it mangles from the driver.
Since introducing child devices would change the userspace interface, a
change event on BUS_NOTIFY_BOUND_DRIVER would probably be the most
reasonable for our busses.
>
> We don't want to encourage any such use model in general, and such
> hacks should be bus/driver specific (and only for legacy reasons), and
> they do not belong into the driver core.
At the end of the day, we just want a working system :)
Cornelia
On Wed, Dec 15, 2010 at 07:08:44PM +0100, Cornelia Huck wrote:
> On Wed, 15 Dec 2010 18:51:48 +0100,
> Kay Sievers <[email protected]> wrote:
>
> > 2010/12/15 Cornelia Huck <[email protected]>:
> > > On Wed, 15 Dec 2010 08:23:16 -0800, Greg KH <[email protected]> wrote:
> > >> On Wed, Dec 15, 2010 at 02:21:13PM +0100, Cornelia Huck wrote:
> >
> > >> How about I turn it around for you, please show me how the driver core
> > >> does _not_ support this today? ?If you can prove that this isn't working
> > >> properly, then great, I'll gladly accept patches to resolve it.
> > >
> > > Looking at device_add():
> >
> > ...
> >
> > > This will not be a problem if a device driver registers a child device
> > > (since it can specify the attributes there).
> >
> > Which is the proper way to do it. No driver should ever mangle a
> > device which it does not own. It's like adding properties of a block
> > device directly to a usb_interface device. That just can not work
> > correctly for many reasons, inside and outside of the kernel.
>
> That's fine for new device drivers.
No, that's for _all_ drivers, why should yours be "special" and not work
this way?
> > > I think the basic problem is that the KOBJ_ADD uevent notifies
> > > userspace that "a device is there", while the device will only be
> > > really useable by userspace once a driver has bound to it.
> >
> > This device represents a device on a bus, and can usually do its own
> > things. A driver can bind to it, but should not mangle it.
> >
> > > A module
> > > load triggered by KOBJ_ADD is fine, but trying to actually use the
> > > device after KOBJ_ADD is racy. This will not matter in the usual case,
> > > since either the matching/probing is fast enough or userspace will wait
> > > for something like a block device anyway, but we've seen problems on
> > > s390. A KOBJ_BIND/UNBIND would make a proper distinction between
> > > "device is there" and "device is usable".
> >
> > We don't rally want any such events. We expect a new child device
> > being created from the driver, instead of re-using the existing bus
> > device.
>
> Do we want to force a device driver to create a child device just to
> notify userspace of the bind?
That's the way all other buses and drivers work, again, why are your
devices and drivers "special" here?
> > > (Besides, what happens on unbind/bind? Shouldn't userspace know that a
> > > device is now bound to a different driver?)
> >
> > It does that by watching the child devices the driver creates and destroys.
> >
> > We already have enough events to handle on today's boxes, we really
> > don't want to add new ones, which are only needed to work around such
> > use cases, which ideally just should be fixed.
> >
> > If you can not change the current drivers to create child devices, the
> > driver can probably just send change events for the already existing
> > devices it mangles from the driver.
>
> Since introducing child devices would change the userspace interface, a
> change event on BUS_NOTIFY_BOUND_DRIVER would probably be the most
> reasonable for our busses.
No, you _already_ get those events, and you can add attributes
automatically when that happens today!
thanks,
greg k-h
On Wed, 15 Dec 2010 10:18:54 -0800,
Greg KH <[email protected]> wrote:
> On Wed, Dec 15, 2010 at 07:08:44PM +0100, Cornelia Huck wrote:
> > On Wed, 15 Dec 2010 18:51:48 +0100,
> > Kay Sievers <[email protected]> wrote:
> >
> > > 2010/12/15 Cornelia Huck <[email protected]>:
> > > > On Wed, 15 Dec 2010 08:23:16 -0800, Greg KH <[email protected]> wrote:
> > > >> On Wed, Dec 15, 2010 at 02:21:13PM +0100, Cornelia Huck wrote:
> > >
> > > >> How about I turn it around for you, please show me how the driver core
> > > >> does _not_ support this today? ?If you can prove that this isn't working
> > > >> properly, then great, I'll gladly accept patches to resolve it.
> > > >
> > > > Looking at device_add():
> > >
> > > ...
> > >
> > > > This will not be a problem if a device driver registers a child device
> > > > (since it can specify the attributes there).
> > >
> > > Which is the proper way to do it. No driver should ever mangle a
> > > device which it does not own. It's like adding properties of a block
> > > device directly to a usb_interface device. That just can not work
> > > correctly for many reasons, inside and outside of the kernel.
> >
> > That's fine for new device drivers.
>
> No, that's for _all_ drivers, why should yours be "special" and not work
> this way?
Because it would break existing userspace.
>
> > > > I think the basic problem is that the KOBJ_ADD uevent notifies
> > > > userspace that "a device is there", while the device will only be
> > > > really useable by userspace once a driver has bound to it.
> > >
> > > This device represents a device on a bus, and can usually do its own
> > > things. A driver can bind to it, but should not mangle it.
> > >
> > > > A module
> > > > load triggered by KOBJ_ADD is fine, but trying to actually use the
> > > > device after KOBJ_ADD is racy. This will not matter in the usual case,
> > > > since either the matching/probing is fast enough or userspace will wait
> > > > for something like a block device anyway, but we've seen problems on
> > > > s390. A KOBJ_BIND/UNBIND would make a proper distinction between
> > > > "device is there" and "device is usable".
> > >
> > > We don't rally want any such events. We expect a new child device
> > > being created from the driver, instead of re-using the existing bus
> > > device.
> >
> > Do we want to force a device driver to create a child device just to
> > notify userspace of the bind?
>
> That's the way all other buses and drivers work, again, why are your
> devices and drivers "special" here?
?? This has nothing to do with our drivers. Why should a generic driver
that doesn't need extra attributes etc. create a child device? (Well,
if that's the way it's supposed to work...)
>
> > > > (Besides, what happens on unbind/bind? Shouldn't userspace know that a
> > > > device is now bound to a different driver?)
> > >
> > > It does that by watching the child devices the driver creates and destroys.
> > >
> > > We already have enough events to handle on today's boxes, we really
> > > don't want to add new ones, which are only needed to work around such
> > > use cases, which ideally just should be fixed.
> > >
> > > If you can not change the current drivers to create child devices, the
> > > driver can probably just send change events for the already existing
> > > devices it mangles from the driver.
> >
> > Since introducing child devices would change the userspace interface, a
> > change event on BUS_NOTIFY_BOUND_DRIVER would probably be the most
> > reasonable for our busses.
>
> No, you _already_ get those events, and you can add attributes
> automatically when that happens today!
But it is still _after_ the KOBJ_ADD uevent, which makes it still racy,
regardless of who adds the attributes. (And of course we would only do
the change event for our busses, not for everyone.)
Cornelia