From: Benny Halevy <bhalevy@primarydata.com>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: bfields@redhat.com, linux-nfs@vger.kernel.org
Subject: Re: [PATCH] nfsd: no need to unhash_stid before free
Date: Tue, 29 Oct 2013 09:00:15 +0200 [thread overview]
Message-ID: <526F5CFF.2080003@primarydata.com> (raw)
In-Reply-To: <20131028183214.GH31322@fieldses.org>
On 2013-10-28 20:32, J. Bruce Fields wrote:
> On Mon, Oct 14, 2013 at 09:01:55AM +0300, Benny Halevy wrote:
>> idr_remove is about to be called before kmem_cache_free so unhashing it
>> is redundant
>
> This leaves only two unhash_stid callers, in release_lock_stateid and
> unhash_stid, both called just before destroying the stateid, so perhaps
> we should remove those and unhash_stid?
In my state lock elimination patchset I actually keep using unhash_stid
after refactoring non-blocking unhashing from the blocking release
so I don't think it's worth it to remove the call.
That said, we can still open code it but encapsulating the assignment
in unhash_stid() which the compiler can inline anyway seems cleaner to me.
Benny
>
> --b.
>
>>
>> Signed-off-by: Benny Halevy <bhalevy@primarydata.com>
>> ---
>> fs/nfsd/nfs4state.c | 7 ++-----
>> 1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 0874998..06984e3 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -668,7 +668,6 @@ static void unhash_open_stateid(struct nfs4_ol_stateid *stp)
>> static void release_open_stateid(struct nfs4_ol_stateid *stp)
>> {
>> unhash_open_stateid(stp);
>> - unhash_stid(&stp->st_stid);
>> free_generic_stateid(stp);
>> }
>>
>> @@ -690,7 +689,6 @@ static void release_last_closed_stateid(struct nfs4_openowner *oo)
>> struct nfs4_ol_stateid *s = oo->oo_last_closed_stid;
>>
>> if (s) {
>> - unhash_stid(&s->st_stid);
>> free_generic_stateid(s);
>> oo->oo_last_closed_stid = NULL;
>> }
>> @@ -3998,10 +3996,9 @@ static void nfsd4_close_open_stateid(struct nfs4_ol_stateid *s)
>>
>> nfsd4_close_open_stateid(stp);
>>
>> - if (cstate->minorversion) {
>> - unhash_stid(&stp->st_stid);
>> + if (cstate->minorversion)
>> free_generic_stateid(stp);
>> - } else
>> + else
>> oo->oo_last_closed_stid = stp;
>>
>> if (list_empty(&oo->oo_owner.so_stateids)) {
>> --
>> 1.8.3.1
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
next prev parent reply other threads:[~2013-10-29 7:00 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-14 6:01 [PATCH] nfsd: no need to unhash_stid before free Benny Halevy
2013-10-28 18:32 ` J. Bruce Fields
2013-10-29 7:00 ` Benny Halevy [this message]
2013-10-29 13:39 ` J. Bruce Fields
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=526F5CFF.2080003@primarydata.com \
--to=bhalevy@primarydata.com \
--cc=bfields@fieldses.org \
--cc=bfields@redhat.com \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).