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: [email protected] [v3.4+]
Signed-off-by: Peng Tao <[email protected]>
---
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)
--
1.9.1
On Wed, Nov 5, 2014 at 8:49 PM, Trond Myklebust
<[email protected]> wrote:
> On Wed, Nov 5, 2014 at 4:00 AM, Peng Tao <[email protected]> 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: [email protected] [v3.4+]
>> Signed-off-by: Peng Tao <[email protected]>
>> ---
>> 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
On Wed, Nov 5, 2014 at 4:00 AM, Peng Tao <[email protected]> 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: [email protected] [v3.4+]
> Signed-off-by: Peng Tao <[email protected]>
> ---
> 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