| Message ID | 1397846704-14567-30-git-send-email-trond.myklebust@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
We're adding a put without adding a corresponding get, so was there a leak before this patch? --b. On Fri, Apr 18, 2014 at 02:44:23PM -0400, Trond Myklebust wrote: > All stateids are associated with a nfs4_file. Let's consolidate... > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > fs/nfsd/nfs4state.c | 2 ++ > fs/nfsd/state.h | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 32ab3f1c83f8..5bbef4720e7c 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -474,6 +474,8 @@ static void remove_stid(struct nfs4_stid *s) > > static void nfs4_free_stid(struct kmem_cache *slab, struct nfs4_stid *s) > { > + if (s->sc_file) > + put_nfs4_file(s->sc_file); > kmem_cache_free(slab, s); > } > > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h > index 9d0088c244a8..c6deef936693 100644 > --- a/fs/nfsd/state.h > +++ b/fs/nfsd/state.h > @@ -85,6 +85,7 @@ struct nfs4_stid { > unsigned char sc_type; > stateid_t sc_stateid; > struct nfs4_client *sc_client; > + struct nfs4_file *sc_file; > }; > > struct nfs4_delegation { > -- > 1.9.0 > -- 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
On Wed, May 7, 2014 at 3:25 PM, Bruce Fields <bfields@fieldses.org> wrote: > We're adding a put without adding a corresponding get, so was there a > leak before this patch? No. The struct sc_file is added by this patch, and since the entire stid is zeroed on allocation (through the use of kmem_cache_zalloc), it is safe to add the put() in this patch. > > --b. > > On Fri, Apr 18, 2014 at 02:44:23PM -0400, Trond Myklebust wrote: >> All stateids are associated with a nfs4_file. Let's consolidate... >> >> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> >> --- >> fs/nfsd/nfs4state.c | 2 ++ >> fs/nfsd/state.h | 1 + >> 2 files changed, 3 insertions(+) >> >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >> index 32ab3f1c83f8..5bbef4720e7c 100644 >> --- a/fs/nfsd/nfs4state.c >> +++ b/fs/nfsd/nfs4state.c >> @@ -474,6 +474,8 @@ static void remove_stid(struct nfs4_stid *s) >> >> static void nfs4_free_stid(struct kmem_cache *slab, struct nfs4_stid *s) >> { >> + if (s->sc_file) >> + put_nfs4_file(s->sc_file); >> kmem_cache_free(slab, s); >> } >> >> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h >> index 9d0088c244a8..c6deef936693 100644 >> --- a/fs/nfsd/state.h >> +++ b/fs/nfsd/state.h >> @@ -85,6 +85,7 @@ struct nfs4_stid { >> unsigned char sc_type; >> stateid_t sc_stateid; >> struct nfs4_client *sc_client; >> + struct nfs4_file *sc_file; >> }; >> >> struct nfs4_delegation { >> -- >> 1.9.0 >>
On Thu, May 08, 2014 at 03:40:49PM -0400, Trond Myklebust wrote: > On Wed, May 7, 2014 at 3:25 PM, Bruce Fields <bfields@fieldses.org> wrote: > > We're adding a put without adding a corresponding get, so was there a > > leak before this patch? > > No. The struct sc_file is added by this patch, and since the entire > stid is zeroed on allocation (through the use of kmem_cache_zalloc), > it is safe to add the put() in this patch. It's safe, but it would still be nicer to have a single patch that just moves the file from the delegation to the generic stateid. -- 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
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 32ab3f1c83f8..5bbef4720e7c 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -474,6 +474,8 @@ static void remove_stid(struct nfs4_stid *s) static void nfs4_free_stid(struct kmem_cache *slab, struct nfs4_stid *s) { + if (s->sc_file) + put_nfs4_file(s->sc_file); kmem_cache_free(slab, s); } diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h index 9d0088c244a8..c6deef936693 100644 --- a/fs/nfsd/state.h +++ b/fs/nfsd/state.h @@ -85,6 +85,7 @@ struct nfs4_stid { unsigned char sc_type; stateid_t sc_stateid; struct nfs4_client *sc_client; + struct nfs4_file *sc_file; }; struct nfs4_delegation {
All stateids are associated with a nfs4_file. Let's consolidate... Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfsd/nfs4state.c | 2 ++ fs/nfsd/state.h | 1 + 2 files changed, 3 insertions(+)