Re: [PATCH] (1/8) Eliminate brlock in psnap

[Paul McKenney]

> On Tue, 11 Mar 2003, Stephen Hemminger wrote:
>
> >  void unregister_snap_client(struct datalink_proto *proto)
> >  {
> > -        br_write_lock_bh(BR_NETPROTO_LOCK);
> > +        static RCU_HEAD(snap_rcu);
> >
> > -        list_del(&proto->node);
> > -        kfree(proto);
> > +        spin_lock_bh(&snap_lock);
> > +        list_del_rcu(&proto->node);
> > +        spin_unlock_bh(&snap_lock);
> >
> > -        br_write_unlock_bh(BR_NETPROTO_LOCK);
> > +        call_rcu(&snap_rcu, (void (*)(void *)) kfree, proto);
> >  }
>
> Do we need the spin_lock_bh around the list_del_rcu? But also How
> about. This way we don't change the previous characteristic of block till
> done unregistering
>
> struct datalink_proto {
> ...
>            struct completion registration;
> };
>
> void __unregister_snap_client(void *__proto)
> {
>            struct datalink_proto *proto = __proto;
>            complete(&proto->registration);
> }
>
> unregister_snap_client(struct datalink_proto *proto)
> {
>            list_del_rcu(&proto->node);
>            call_rcu(&snap_rcu, __unregister_snap_client, proto);
>            wait_for_completion(&proto->registration);
>            kfree(proto);
> }

You are saying that we can omit locking because this is
called only from a module-exit function, and thus protected
by the module_mutex semaphore?  (I must defer to
others who have a better handle on modules...)

If in fact only one module-exit function can be
executing at a given time, then we should be able to
use the following approach:

void unregister_snap_client(struct datalink_proto *proto)
{
      list_del_rcu(&proto->node); /* protected by module_mutex. */
      synchronize_kernel();
      kfree(proto);
}

If multiple module-exit functions can somehow execute
concurrently, then something like the following would
be needed:

void unregister_snap_client(struct datalink_proto *proto)
{
      spin_lock_bh(&snap_lock);
      list_del_rcu(&proto->node);
      spin_unlock_bh(&snap_lock);
      synchronize_kernel();
      kfree(proto);
}

Module unloading should be rare enough to tolerate
the grace period under the module_mutex, right?

Thoughts?

                        Thanx, Paul