| Message ID | 1415178014-17893-1-git-send-email-tao.peng@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 5, 2014 at 4:00 AM, Peng Tao <tao.peng@primarydata.com> wrote: > For pNFS direct writes, layout driver may dynamically allocate ds_cinfo.buckets. > So we need to take care to free them when freeing dreq. > > Ideally this needs to be done inside layout driver where ds_cinfo.buckets > are allocated. But buckets are attached to dreq and reused across LD IO iterations. > So I feel it's OK to free them in the generic layer. > > Cc: stable@vger.kernel.org [v3.4+] > Signed-off-by: Peng Tao <tao.peng@primarydata.com> > --- > fs/nfs/direct.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c > index 20cffc8..61f1904 100644 > --- a/fs/nfs/direct.c > +++ b/fs/nfs/direct.c > @@ -266,6 +266,8 @@ static void nfs_direct_req_free(struct kref *kref) > { > struct nfs_direct_req *dreq = container_of(kref, struct nfs_direct_req, kref); > > + if (dreq->ds_cinfo.nbuckets) > + kfree(dreq->ds_cinfo.buckets); > if (dreq->l_ctx != NULL) > nfs_put_lock_context(dreq->l_ctx); > if (dreq->ctx != NULL) > -- > Well spotted! However doesn't the above need to be limited with an #ifdef CONFIG_NFS_V4_1? Perhaps you can add a helper in include/linux/nfs_xdr.h that takes a struct pnfs_ds_commit_info and then frees the nbuckets. Note also that kfree() is happy to take a NULL argument. Thanks! Trond -- 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, Nov 5, 2014 at 8:49 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > On Wed, Nov 5, 2014 at 4:00 AM, Peng Tao <tao.peng@primarydata.com> wrote: >> For pNFS direct writes, layout driver may dynamically allocate ds_cinfo.buckets. >> So we need to take care to free them when freeing dreq. >> >> Ideally this needs to be done inside layout driver where ds_cinfo.buckets >> are allocated. But buckets are attached to dreq and reused across LD IO iterations. >> So I feel it's OK to free them in the generic layer. >> >> Cc: stable@vger.kernel.org [v3.4+] >> Signed-off-by: Peng Tao <tao.peng@primarydata.com> >> --- >> fs/nfs/direct.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c >> index 20cffc8..61f1904 100644 >> --- a/fs/nfs/direct.c >> +++ b/fs/nfs/direct.c >> @@ -266,6 +266,8 @@ static void nfs_direct_req_free(struct kref *kref) >> { >> struct nfs_direct_req *dreq = container_of(kref, struct nfs_direct_req, kref); >> >> + if (dreq->ds_cinfo.nbuckets) >> + kfree(dreq->ds_cinfo.buckets); >> if (dreq->l_ctx != NULL) >> nfs_put_lock_context(dreq->l_ctx); >> if (dreq->ctx != NULL) >> -- >> > > Well spotted! > However doesn't the above need to be limited with an #ifdef > CONFIG_NFS_V4_1? Perhaps you can add a helper in > include/linux/nfs_xdr.h that takes a struct pnfs_ds_commit_info and > then frees the nbuckets. > Note also that kfree() is happy to take a NULL argument. > You are right! I'll send v2. Thanks, Tao -- 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/nfs/direct.c b/fs/nfs/direct.c index 20cffc8..61f1904 100644 --- a/fs/nfs/direct.c +++ b/fs/nfs/direct.c @@ -266,6 +266,8 @@ static void nfs_direct_req_free(struct kref *kref) { struct nfs_direct_req *dreq = container_of(kref, struct nfs_direct_req, kref); + if (dreq->ds_cinfo.nbuckets) + kfree(dreq->ds_cinfo.buckets); if (dreq->l_ctx != NULL) nfs_put_lock_context(dreq->l_ctx); if (dreq->ctx != NULL)
For pNFS direct writes, layout driver may dynamically allocate ds_cinfo.buckets. So we need to take care to free them when freeing dreq. Ideally this needs to be done inside layout driver where ds_cinfo.buckets are allocated. But buckets are attached to dreq and reused across LD IO iterations. So I feel it's OK to free them in the generic layer. Cc: stable@vger.kernel.org [v3.4+] Signed-off-by: Peng Tao <tao.peng@primarydata.com> --- fs/nfs/direct.c | 2 ++ 1 file changed, 2 insertions(+)