Re: [PATCH RFD] alternative kobject release wait mechanism

On Thu, 19 Apr 2007, Dmitry Torokhov wrote:

On 4/19/07, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
On Wed, 18 Apr 2007, Dmitry Torokhov wrote:

I am still do not understand why this is needed. Would it not be
simplier just to use a reference to struct device instead of embedding
it in a larger structure if their lifetimes are different and one does
not have a subsystem that takes care of releasing logic.


Pretty much drivers have 2 options:

struct my_device {
void *private_data;
struct device dev;
};

Actually people use dev_[gs]et_drvdata() instead of a separate
private_data pointer. That way there's no need for the my_device
container.


No, drvdata belongs to driver that is bound to a device. We are
talking about private data created and managed by driver that provides
device.

Oh, sorry, I misunderstood.

Again, if we are talking about driver bound to a device then devices
->release() is irrelevant. If we are talking about driver that created
device then driver's ->remove() is irrelevant.

The problem is that device structures carry references to their parents.
So even if a driver does use this option and is able to give up its own
references immediately, it can't make any guarantees about drivers of
child devices. If every driver followed your scheme we'd be okay; until
then any ancestor of a device with a non-immediate-detach driver would not
be able to do immediate detach.

There's also another problem. Even though all the references to
my_dev->dev may be gone, there will probably still be outstanding
references to private_data structure. The driver's exit() routine will
have to wait until all those references are gone. How can it do that
safely? Presumably the private_data will include its own refcount and
some release routine will run when the count drops to 0. That routine
will be part of the driver's module -- so how can it enable itself to be
unloaded?


With current sysfs orphaning attributes upon removal request there is
no issue of accessing driver-private data through references obtained
via ether embedded or referenced dev structure so everything is fine.

Not so. There are other pathways besides sysfs which can cause a driver
to access its data structures.


Which ones? These needs to be identified and treated with "immediate
disconnect" that you advocated earlier. Once active users of device's
services are gone you can zap it.

Among the worst offenders are character devices. None of the subsystem
providers offering char device registration performs immediate detach --
they are a lot like sysfs used to be. (In fact, they probably _can't_
provide it since read() or write() calls can block indefinitely in the
lower-level driver; the subsystem may have no way to force them to
fail-fast.) So every lower-level driver which registers a char device
needs to provide its own synchronization and immediate detach, and at the
moment I doubt very many of them do. And of those which do, a large
percentage rely on the BKL for synchronization!

Here's another awkward possibility. Suppose a workqueue routine tries to
unregister a device. Suppose the device's driver needs to do something in
a process context, so it has a work item pending in the same workqueue.
The driver's remove() method would need to flush the workqueue (in order
to perform immediate detach), but doing so would cause a deadlock.

Alan Stern

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • [PATCH v2] v4l2_device/v4l2_subdev: final (?) version
    ... +Overview of the V4L2 driver framework ... +1) A struct for each device instance containing the device state. ... +of which only a handful of ops are commonly implemented, ... +may be NULL if the subdev driver does not support anything from that category. ...
    (Linux-Kernel)
  • [PATCH] [TRIVIAL] Fixing occurrences of "the the "
    ... Registering a driver using platform_driver_probeworks just like ... Free all pages associated with DMA buffers, the buffers and pages lists, and ... * hw - Struct containing variables accessed by shared code ... * This option is used to tune the the maximum retransmission attempts ...
    (Linux-Kernel)
  • Re: [PATCH] [TRIVIAL] Fixing occurrences of "the the "
    ... Registering a driver using platform_driver_probeworks just like ... Free all pages associated with DMA buffers, the buffers and pages lists, and ... * hw - Struct containing variables accessed by shared code ... * This option is used to tune the the maximum retransmission attempts ...
    (Linux-Kernel)
  • Re: v4l2_device/v4l2_subdev: please review (PATCH 1/3)
    ... changes are just boring i2c driver conversions. ... +1) A struct for each device instance containing the device state. ... +of which only a handful of ops are commonly implemented, ... +may be NULL if the subdev driver does not support anything from that category. ...
    (Linux-Kernel)
  • Re: [PATCH V2] ipwireless_cs driver for 4G PC Card
    ... +IPWIRELESS DRIVER ... +static struct timing_stats { ... +static void end_read_timing ... +/* Number of bytes in NL packet header (can not do ...
    (Linux-Kernel)
Loading