On Wed, Nov 5, 2014 at 8:49 PM, Trond Myklebust
<trond.myklebust@xxxxxxxxxxxxxxx> wrote:
> On Wed, Nov 5, 2014 at 4:00 AM, Peng Tao <tao.peng@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx [v3.4+]
>> Signed-off-by: Peng Tao <tao.peng@xxxxxxxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
