Messages in this thread | | | Subject | Re: [PATCH 2/2] auth_gss: unregister gss_domain when unloading module | From | Trond Myklebust <> | Date | Mon, 30 Oct 2006 10:17:29 -0500 |
| |
On Mon, 2006-10-30 at 23:54 +0900, Akinobu Mita wrote: > On Sun, Oct 29, 2006 at 03:15:59PM -0500, Trond Myklebust wrote: > > > > +void svcauth_gss_unregister_pseudoflavor(char *name) > > > +{ > > > + struct auth_domain *dom; > > > + > > > + dom = auth_domain_find(name); > > > + if (dom) { > > > + auth_domain_put(dom); > > > + auth_domain_put(dom); > > > + } > > > +} > > > > Strictly speaking, if you want to be smp-safe, you probably need > > something like the following: > > > > dom = auth_domain_find(name); > > if (dom) { > > spin_lock(&auth_domain_lock); > > if (!hlist_unhashed(dom->hash)) { > > hlist_del_init(dom->hash); > > spin_unlock(&auth_domain_lock); > > auth_domain_put(dom); > > } else > > spin_unlock(&auth_domain_lock); > > auth_domain_put(dom); > > } > > > > and then add a test for hlist_unhashed into auth_domain_put(). If not, > > some other processor could race you inside > > svcauth_gss_unregister_pseudoflavor. > > But auth_domain_table is protected by auth_domain_lock while we are > using auth_domain_put()/auth_domain_lookup()/auth_domain_find(). > So I think there is not big difference.
No. The auth_domain_lock was released after the call to auth_domain_find(), and thus there is no guarantee that the entry is still referenced when you get round to that second call to auth_domain_put(). Testing for hlist_unhashed() and then removing the entry from the lookup table while under the spin lock fixes this problem: it ensures that you only call auth_domain_put() once if some other process has raced you.
Actually, making a helper "auth_domain_find_and_unhash()" would probably be even more efficient in this case, since that would enable you to reduce the auth_domain_lock usage further.
Cheers, Trond - 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/
| |