On Mon, Mar 30, 2009 at 11:34:32PM +0300, Benny Halevy wrote:
> On Mar. 30, 2009, 23:08 +0300, "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote:
> > On Sat, Mar 28, 2009 at 11:31:50AM +0300, Benny Halevy wrote:
> >> + return NULL;
> >> +}
> >> +
> >> +/* caller must hold sessionid_lock */
> >> static void
> >> -release_session(struct nfsd4_session *ses)
> >> +unhash_session(struct nfsd4_session *ses)
> >> {
> >> list_del(&ses->se_hash);
> >> list_del(&ses->se_perclnt);
> >> +}
> >> +
> >> +static void
> >> +release_session(struct nfsd4_session *ses)
> >> +{
> >> + spin_lock(&sessionid_lock);
> >> + unhash_session(ses);
> >> + spin_unlock(&sessionid_lock);
> >> nfsd4_put_session(ses);
> >> }
> >
> > It's not obvious from the names what the difference between
> > release_session() and nfsd4_put_session() is.
> >
> > How about just renaming release_session() to unhash_session(), and
> > dumping hash_session? The two list_del()'s don't need their own
> > function.
>
> We call unhash_session on its own later on from destroy_session,
> then we destroy the callback client and finally put the session.
Oops, I see, I missed that.
>
> We can embed release_session into expire_client since it's
> its only use though expire_client is hairy enough I'm not
> sure we want to add more stuff into it. If we're going
> this direction, I'd consider refactoring it and taking
> the many loops it's doing out into their own functions.
> (we'll add a couple more for pNFS - for releasing layouts
> and layoutrecalls)
Sounds OK.--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
