From: Andy Adamson <[email protected]>
Do not return an error when nfs4_copy_delegation_stateid succeeds.
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/nfs4state.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index ba18958..1cfde97 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1120,8 +1120,11 @@ int nfs4_select_rw_stateid(nfs4_stateid *dst, struct nfs4_state *state,
if (ret == -EIO)
/* A lost lock - don't even consider delegations */
goto out;
- if (nfs4_copy_delegation_stateid(dst, state->inode, fmode))
+ /* returns true if delegation stateid found and copied */
+ if (nfs4_copy_delegation_stateid(dst, state->inode, fmode)) {
+ ret = 0;
goto out;
+ }
if (ret != -ENOENT)
/* nfs4_copy_delegation_stateid() didn't over-write
* dst, so it still has the lock stateid which we now
--
1.8.2.1
On Tue, 18 Feb 2014 11:33:42 -0500 Trond Myklebust
<[email protected]> wrote:
> On Tue, 2014-02-18 at 10:36 -0500, [email protected] wrote:
> > From: Andy Adamson <[email protected]>
> >
> > Do not return an error when nfs4_copy_delegation_stateid succeeds.
> >
> > Signed-off-by: Andy Adamson <[email protected]>
> > ---
> > fs/nfs/nfs4state.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> > index ba18958..1cfde97 100644
> > --- a/fs/nfs/nfs4state.c
> > +++ b/fs/nfs/nfs4state.c
> > @@ -1120,8 +1120,11 @@ int nfs4_select_rw_stateid(nfs4_stateid *dst, struct nfs4_state *state,
> > if (ret == -EIO)
> > /* A lost lock - don't even consider delegations */
> > goto out;
> > - if (nfs4_copy_delegation_stateid(dst, state->inode, fmode))
> > + /* returns true if delegation stateid found and copied */
> > + if (nfs4_copy_delegation_stateid(dst, state->inode, fmode)) {
> > + ret = 0;
> > goto out;
> > + }
> > if (ret != -ENOENT)
> > /* nfs4_copy_delegation_stateid() didn't over-write
> > * dst, so it still has the lock stateid which we now
>
> Ouch! That looks like it would trigger looping in both the read and
> write code when we're holding a delegation. Is that what you end up
> seeing?
>
> It looks like it was introduced by commit ef1820f9be27b...
>
Oops- that was a little careless. Thanks for the heads-up!
NeilBrown
On Feb 18, 2014, at 11:33 AM, Trond Myklebust <[email protected]> wrote:
> On Tue, 2014-02-18 at 10:36 -0500, [email protected] wrote:
>> From: Andy Adamson <[email protected]>
>>
>> Do not return an error when nfs4_copy_delegation_stateid succeeds.
>>
>> Signed-off-by: Andy Adamson <[email protected]>
>> ---
>> fs/nfs/nfs4state.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
>> index ba18958..1cfde97 100644
>> --- a/fs/nfs/nfs4state.c
>> +++ b/fs/nfs/nfs4state.c
>> @@ -1120,8 +1120,11 @@ int nfs4_select_rw_stateid(nfs4_stateid *dst, struct nfs4_state *state,
>> if (ret == -EIO)
>> /* A lost lock - don't even consider delegations */
>> goto out;
>> - if (nfs4_copy_delegation_stateid(dst, state->inode, fmode))
>> + /* returns true if delegation stateid found and copied */
>> + if (nfs4_copy_delegation_stateid(dst, state->inode, fmode)) {
>> + ret = 0;
>> goto out;
>> + }
>> if (ret != -ENOENT)
>> /* nfs4_copy_delegation_stateid() didn't over-write
>> * dst, so it still has the lock stateid which we now
>
> Ouch! That looks like it would trigger looping in both the read and
> write code when we're holding a delegation. Is that what you end up
> seeing?
Actually, I saw it trying to run Anna?s intra server to server copy code. In this case, the server side copy failed and normal READ/WRITE from the client then suceeded. In that case I did not see any looping...
>
> It looks like it was introduced by commit ef1820f9be27b?
Yep. Should I include that commit in the comment?
?>Andy
>
> --
> Trond Myklebust
> Linux NFS client maintainer, PrimaryData
> [email protected]
On Tue, 2014-02-18 at 17:56 +0000, Adamson, Andy wrote:
> On Feb 18, 2014, at 11:33 AM, Trond Myklebust <[email protected]> wrote:
>
> > On Tue, 2014-02-18 at 10:36 -0500, [email protected] wrote:
> >> From: Andy Adamson <[email protected]>
> >>
> >> Do not return an error when nfs4_copy_delegation_stateid succeeds.
> >>
> >> Signed-off-by: Andy Adamson <[email protected]>
> >> ---
> >> fs/nfs/nfs4state.c | 5 ++++-
> >> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> >> index ba18958..1cfde97 100644
> >> --- a/fs/nfs/nfs4state.c
> >> +++ b/fs/nfs/nfs4state.c
> >> @@ -1120,8 +1120,11 @@ int nfs4_select_rw_stateid(nfs4_stateid *dst, struct nfs4_state *state,
> >> if (ret == -EIO)
> >> /* A lost lock - don't even consider delegations */
> >> goto out;
> >> - if (nfs4_copy_delegation_stateid(dst, state->inode, fmode))
> >> + /* returns true if delegation stateid found and copied */
> >> + if (nfs4_copy_delegation_stateid(dst, state->inode, fmode)) {
> >> + ret = 0;
> >> goto out;
> >> + }
> >> if (ret != -ENOENT)
> >> /* nfs4_copy_delegation_stateid() didn't over-write
> >> * dst, so it still has the lock stateid which we now
> >
> > Ouch! That looks like it would trigger looping in both the read and
> > write code when we're holding a delegation. Is that what you end up
> > seeing?
>
> Actually, I saw it trying to run Anna’s intra server to server copy code. In this case, the server side copy failed and normal READ/WRITE from the client then suceeded. In that case I did not see any looping...
>
> >
> > It looks like it was introduced by commit ef1820f9be27b…
>
> Yep. Should I include that commit in the comment?
I added a 'Fixes:' and 'Link:' to the patch before I applied it. I also
added a stable fix request for 3.12+...
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
[email protected]
On Tue, 2014-02-18 at 10:36 -0500, [email protected] wrote:
> From: Andy Adamson <[email protected]>
>
> Do not return an error when nfs4_copy_delegation_stateid succeeds.
>
> Signed-off-by: Andy Adamson <[email protected]>
> ---
> fs/nfs/nfs4state.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index ba18958..1cfde97 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -1120,8 +1120,11 @@ int nfs4_select_rw_stateid(nfs4_stateid *dst, struct nfs4_state *state,
> if (ret == -EIO)
> /* A lost lock - don't even consider delegations */
> goto out;
> - if (nfs4_copy_delegation_stateid(dst, state->inode, fmode))
> + /* returns true if delegation stateid found and copied */
> + if (nfs4_copy_delegation_stateid(dst, state->inode, fmode)) {
> + ret = 0;
> goto out;
> + }
> if (ret != -ENOENT)
> /* nfs4_copy_delegation_stateid() didn't over-write
> * dst, so it still has the lock stateid which we now
Ouch! That looks like it would trigger looping in both the read and
write code when we're holding a delegation. Is that what you end up
seeing?
It looks like it was introduced by commit ef1820f9be27b...
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
[email protected]