On Mar. 30, 2009, 23:59 +0300, "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote:
> 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.
OK. I'll send a cleanup patch that will bring the code into
the final version. Eventually it'll split into one patch against
for-2.6.30 to refactor expire_client and the rest will be squashed
here. (plus the changes will percolate through to the pnfs branch)
Benny
--
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
