Re: Driverfs updates

[Keith Owens <kaos@ocs.com.au>]

On Mon, 8 Jul 2002 17:52:13 -0600 (MDT), 
Thunder from the hill <thunder@ngforever.de> wrote:
>Hi,
>
>On Tue, 9 Jul 2002, Keith Owens wrote:
>> struct device_driver * get_driver(struct device_driver * drv)
>> {
> +        struct device_driver *ret = NULL;
> +
> +        if (!drv)
> +                goto out;
> +        lock_somehow(drv->lock);
> +        if (drv->owner)
>>                 if (!try_inc_mod_count(drv->owner))
> +                        goto out;
> +
> +        ret = drv;
> + out:
> +        unlock_somehow(drv->lock);
> +        return ret;
>> }
>> 
>> I suggest you add a global driverfs_lock.
>
>Better than locking all kernel threads, isn't it?

What protects drv in that code?  drv is a dynamically registered object
and can be dynamically unregistered and freed at any time from another
cpu, or even the same cpu with preempt.  Any reference to drv without
an external lock or a reference count on the module that registered drv
is racy.  In particular, you cannot use drv->anything to protect drv!

The global driverfs_lock is required to protect the bus/drv list
against changes while you are processing an entry on the list AND that
entry is in a module with a use count of 0.  In that state, you have an
uncounted reference to module data which must be protected until you
can set the use count, at which point the use count will take over and
protect the structure.

Did I mention that this method is complex and fragile?