[pnfs] [PATCH 9/9] pnfs client prevent race in sequence slot

[Benny Halevy]

Hmm Andy,

Actually I'm not quite sure I understand the bug and your solution...
What prevents sequence->list.next from changing after sequence->lock is released?
Similar to what could have happened before you moved the lock, in the goto out path...

Benny


Benny Halevy wrote:
> andros at umich.edu wrote:
>> From: Andy Adamson <andros at umich.edu>
>>
>> Move the lock to cover reading the sequence list in rpc_sequence
>> which allowed multiple rpc's to be in flight with the same slot
>> sequence id.
>>
>> Signed-off by: Andy Adamson<andros at umich.edu>
>> ---
>>  fs/nfs/nfs4state.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
>> index 4963454..cbc08af 100644
>> --- a/fs/nfs/nfs4state.c
>> +++ b/fs/nfs/nfs4state.c
>> @@ -773,15 +773,15 @@ int nfs_wait_on_sequence(struct nfs_seqid *seqid, struct rpc_task *task)
>>  	struct rpc_sequence *sequence = seqid->sequence->sequence;
>>  	int status = 0;
>>  
>> +	spin_lock(&sequence->lock);
>>  	if (sequence->list.next == &seqid->list)
>>  		goto out;
> 
> This statement can be removed now as its else case is
> handled right below. see patch below.
> 
>> -	spin_lock(&sequence->lock);
>>  	if (sequence->list.next != &seqid->list) {
>>  		rpc_sleep_on(&sequence->wait, task, NULL, NULL);
>>  		status = -EAGAIN;
>>  	}
>> -	spin_unlock(&sequence->lock);
>>  out:
>> +	spin_unlock(&sequence->lock);
>>  	return status;
>>  }
>>  
> 
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index cbc08af..5cbdbc0 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -774,13 +774,10 @@ int nfs_wait_on_sequence(struct nfs_seqid *seqid, struct rpc_task *task)
>         int status = 0;
> 
>         spin_lock(&sequence->lock);
> -       if (sequence->list.next == &seqid->list)
> -               goto out;
>         if (sequence->list.next != &seqid->list) {
>                 rpc_sleep_on(&sequence->wait, task, NULL, NULL);
>                 status = -EAGAIN;
>         }
> -out:
>         spin_unlock(&sequence->lock);
>         return status;
>  }
> --
> 
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs