Re: [OKS] Module removal

[Kevin O'Connor]

On Thu, Jul 04, 2002 at 03:29:29AM -0300, Werner Almesberger wrote:
> This certainly seems to be the most understandable way, yes. I think
> modules follow basically the principle illistrated below (that's case
> 4b in the taxonomy I posted earlier), where deregistration doesn't
> stop everything, but which should be safe except for
> return-after-removal:
> 
> foo_dispatch(...)
> {
>     read_lock(global_foo_lock);
>     ...
>     xxx = find_stuff();
>     ...
>     xxx->whatever_op(xxx->data);
> }
> 
> foo_deregister(...)
> {
>     write_lock(global_foo_lock);
>     ...
>     write_unlock(global_foo_lock);
> }
> 
> bar_whatever_op(my_data)
> {   
>     lock(my_data); /* think MOD_INC_USE_COUNT */
>     ...
>     read_unlock(global_foo_lock);
>     ...
>     unlock(my_data); /* think MOD_DEC_USE_COUNT */
>     /* return-after-removal race if my_data also protects code,
>        not only data ! */
> }
> 
> bar_main(...)
> {
>     foo_register(&bar_ops,&bar_data);
>     ...
>     foo_deregister(&bar_ops);
>     ...
>     lock(bar_data); /* barrier */
>     unlock(bar_data);
>     ...
>     destroy(&bar_data);
> }

Hi Werner,

I think the above works, but the locking is a bit hairy.  How about the
following.  (Taxonomy 3)

foo_dispatch(...)
{
    spin_lock(global_foo_lock);
    xxx = find_stuff();            // atomic_inc(xxx->refcnt)
    spin_unlock(global_foo_lock);
    ...
    xxx->whatever_op(xxx->data);
    ...
    put_stuff(xxx);                // atomic_dec(xxx->refcnt)
}

foo_deregister(...)
{
    spin_lock(global_foo_lock);
    remove_stuff(xxx);
    spin_unlock(global_foo_lock);

    while (atomic_read(xxx->refcnt))
        schedule();
}

bar_whatever_op(my_data)
{   
    /* no return-after-removal race because bar is pinned. */
}

bar_main(...)
{
    foo_register(&bar_ops,&bar_data);
    ...
    foo_deregister(&bar_ops);

    /* Now guaranteed that no references to bar_ops exist, and guaranteed
       that no concurrent accesses exist. */

    destroy(&bar_data);
}

I get the feeling you already had something like this in mind, but your
example above was rather complicated.  The latter implementation is just
standard reference counting.

> [...]
> I can understand why people don't want to use totally "safe"
> deregistration, e.g.
> 
>  - locking gets more complex and you may run into hairy deadlock
>    scenarios
>  - accomplishing timely removal may become more difficult
>  - nobody likes patches that change the world

With a standard reference counting implementation I don't see any of these
issues being a problem..

Did I miss something?
-Kevin

-- 
 ------------------------------------------------------------------------
 | Kevin O'Connor                     "BTW, IMHO we need a FAQ for      |
 | [e-mail]                  'IMHO', 'FAQ', 'BTW', etc. !"    |
 ------------------------------------------------------------------------