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

Paul McKenney (Paul.McKenney@us.ibm.com)
Wed, 12 Mar 2003 12:28:12 -0800


> 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

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