2014-10-13 15:25:30

by Trond Myklebust

Subject: Re: [PATCH 1/1] NFSv4: open context can be NULL in nfs_put_open_context

Resending to list... Apologies for the duplicate...

On Mon, Oct 13, 2014 at 11:24 AM, Trond Myklebust
<[email protected]> wrote:
> Hi Andy,
>
>
> On Mon, Oct 13, 2014 at 10:08 AM, <[email protected]> wrote:
>>
>> From: Andy Adamson <[email protected]>
>>
>> If a pgio error is caught before the nfs_pgio_header args->context is set
>> with get_nfs_open_context, the context can be NULL.
>>
>> Signed-off-by: Andy Adamson <[email protected]>
>> ---
>> fs/nfs/inode.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
>> index 141c9f4..a2b148e 100644
>> --- a/fs/nfs/inode.c
>> +++ b/fs/nfs/inode.c
>> @@ -798,6 +798,9 @@ static void __put_nfs_open_context(struct
>> nfs_open_context *ctx, int is_sync)
>> struct inode *inode = ctx->dentry->d_inode;
>> struct super_block *sb = ctx->dentry->d_sb;
>>
>> + if (ctx == NULL)
>> + return;
>> +
>> if (!list_empty(&ctx->list)) {
>> if (!atomic_dec_and_lock(&ctx->lock_context.count,
>> &inode->i_lock))
>> return;
>> --
>> 1.8.3.1
>>
>
> AFAICS, there is only one callsite where we need to check the context, so we
> can avoid the check on all calls to put_nfs_open_context. Please see PATCH
> 1/2 that I just posted in response to an off-list bug report from Steve
> Dickson.
>
> Cheers
> Trond
> --
>
> Trond Myklebust
>
> Linux NFS client maintainer, PrimaryData
>
> [email protected]



--
Trond Myklebust

Linux NFS client maintainer, PrimaryData

[email protected]

2014-10-13 15:28:39

by Weston Andros Adamson

Subject: Re: [PATCH 1/1] NFSv4: open context can be NULL in nfs_put_open_context

On Oct 13, 2014, at 11:25 AM, Trond Myklebust <[email protected]> wrote:

> Resending to list... Apologies for the duplicate...
>
> On Mon, Oct 13, 2014 at 11:24 AM, Trond Myklebust
> <[email protected]> wrote:
>> Hi Andy,
>>
>>
>> On Mon, Oct 13, 2014 at 10:08 AM, <[email protected]> wrote:
>>>
>>> From: Andy Adamson <[email protected]>
>>>
>>> If a pgio error is caught before the nfs_pgio_header args->context is set
>>> with get_nfs_open_context, the context can be NULL.
>>>
>>> Signed-off-by: Andy Adamson <[email protected]>
>>> ---
>>> fs/nfs/inode.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
>>> index 141c9f4..a2b148e 100644
>>> --- a/fs/nfs/inode.c
>>> +++ b/fs/nfs/inode.c
>>> @@ -798,6 +798,9 @@ static void __put_nfs_open_context(struct
>>> nfs_open_context *ctx, int is_sync)
>>> struct inode *inode = ctx->dentry->d_inode;
>>> struct super_block *sb = ctx->dentry->d_sb;
>>>
>>> + if (ctx == NULL)
>>> + return;
>>> +
>>> if (!list_empty(&ctx->list)) {
>>> if (!atomic_dec_and_lock(&ctx->lock_context.count,
>>> &inode->i_lock))
>>> return;
>>> --
>>> 1.8.3.1
>>>
>>
>> AFAICS, there is only one callsite where we need to check the context, so we
>> can avoid the check on all calls to put_nfs_open_context. Please see PATCH
>> 1/2 that I just posted in response to an off-list bug report from Steve
>> Dickson.

Thanks Trond,

Both patches should fix the OOPS, but I think your reasoning to avoid the check in
all calls to put_nfs_open_context makes sense.

-dros

>>
>> Cheers
>> Trond
>> --
>>
>> Trond Myklebust
>>
>> Linux NFS client maintainer, PrimaryData
>>
>> [email protected]
>
>
>
> --
> Trond Myklebust
>
> Linux NFS client maintainer, PrimaryData
>
> [email protected]
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html