| Message ID | 1419021845-96747-2-git-send-email-trond.myklebust@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
This is the patch that breaks recovery of opens upon server reboot. I have a test that open a file and gets a write delegation and tried to do a write. At this point, I reboot my server so that write fails with bad_session and then stale_clientid. Upon completing the exchange_id, create_session, and putrootfh, the client no longer sends the open to be recovered and instead resends the failed write. It would use all 0s stateid (this is 4.1) for the write and for the close that follows it. Reverting the patch fixes the problem. On Fri, Dec 19, 2014 at 3:44 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > Remove an incorrect check for NFS_DELEGATION_NEED_RECLAIM in > can_open_delegated(). We are allowed to cache opens even in > a situation where we're doing reboot recovery. > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > fs/nfs/nfs4proc.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index 8514b59a8c30..002c7dfedb08 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -1119,8 +1119,6 @@ static int can_open_delegated(struct nfs_delegation *delegation, fmode_t fmode) > return 0; > if ((delegation->type & fmode) != fmode) > return 0; > - if (test_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags)) > - return 0; > if (test_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) > return 0; > nfs_mark_delegation_referenced(delegation); > -- > 2.1.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 18, 2015 at 1:38 PM, Olga Kornievskaia <aglo@umich.edu> wrote: > > This is the patch that breaks recovery of opens upon server reboot. > > I have a test that open a file and gets a write delegation and tried > to do a write. At this point, I reboot my server so that write fails > with bad_session and then stale_clientid. Upon completing the > exchange_id, create_session, and putrootfh, the client no longer sends > the open to be recovered and instead resends the failed write. It > would use all 0s stateid (this is 4.1) for the write and for the close > that follows it. Why is the client not attempting to recover the open? That's a bug, whether or not this patch is correct. > On Fri, Dec 19, 2014 at 3:44 PM, Trond Myklebust > <trond.myklebust@primarydata.com> wrote: > > Remove an incorrect check for NFS_DELEGATION_NEED_RECLAIM in > > can_open_delegated(). We are allowed to cache opens even in > > a situation where we're doing reboot recovery. > > > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > > --- > > fs/nfs/nfs4proc.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > > index 8514b59a8c30..002c7dfedb08 100644 > > --- a/fs/nfs/nfs4proc.c > > +++ b/fs/nfs/nfs4proc.c > > @@ -1119,8 +1119,6 @@ static int can_open_delegated(struct nfs_delegation *delegation, fmode_t fmode) > > return 0; > > if ((delegation->type & fmode) != fmode) > > return 0; > > - if (test_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags)) > > - return 0; > > if (test_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) > > return 0; > > nfs_mark_delegation_referenced(delegation); > > -- > > 2.1.0 > > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 18, 2015 at 5:07 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > On Tue, Aug 18, 2015 at 1:38 PM, Olga Kornievskaia <aglo@umich.edu> wrote: >> >> This is the patch that breaks recovery of opens upon server reboot. >> >> I have a test that open a file and gets a write delegation and tried >> to do a write. At this point, I reboot my server so that write fails >> with bad_session and then stale_clientid. Upon completing the >> exchange_id, create_session, and putrootfh, the client no longer sends >> the open to be recovered and instead resends the failed write. It >> would use all 0s stateid (this is 4.1) for the write and for the close >> that follows it. > > Why is the client not attempting to recover the open? That's a bug, > whether or not this patch is correct. I added printks in the path of nfs4_do_reclaim(). Client does "attempt" to recover the open. OPEN never goes on the wire because can_open_delegated() returned true and no action of adding an rpc task gets done. > >> On Fri, Dec 19, 2014 at 3:44 PM, Trond Myklebust >> <trond.myklebust@primarydata.com> wrote: >> > Remove an incorrect check for NFS_DELEGATION_NEED_RECLAIM in >> > can_open_delegated(). We are allowed to cache opens even in >> > a situation where we're doing reboot recovery. >> > >> > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> >> > --- >> > fs/nfs/nfs4proc.c | 2 -- >> > 1 file changed, 2 deletions(-) >> > >> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >> > index 8514b59a8c30..002c7dfedb08 100644 >> > --- a/fs/nfs/nfs4proc.c >> > +++ b/fs/nfs/nfs4proc.c >> > @@ -1119,8 +1119,6 @@ static int can_open_delegated(struct nfs_delegation *delegation, fmode_t fmode) >> > return 0; >> > if ((delegation->type & fmode) != fmode) >> > return 0; >> > - if (test_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags)) >> > - return 0; >> > if (test_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) >> > return 0; >> > nfs_mark_delegation_referenced(delegation); >> > -- >> > 2.1.0 >> > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 18, 2015 at 2:13 PM, Olga Kornievskaia <aglo@umich.edu> wrote: > On Tue, Aug 18, 2015 at 5:07 PM, Trond Myklebust > <trond.myklebust@primarydata.com> wrote: >> On Tue, Aug 18, 2015 at 1:38 PM, Olga Kornievskaia <aglo@umich.edu> wrote: >>> >>> This is the patch that breaks recovery of opens upon server reboot. >>> >>> I have a test that open a file and gets a write delegation and tried >>> to do a write. At this point, I reboot my server so that write fails >>> with bad_session and then stale_clientid. Upon completing the >>> exchange_id, create_session, and putrootfh, the client no longer sends >>> the open to be recovered and instead resends the failed write. It >>> would use all 0s stateid (this is 4.1) for the write and for the close >>> that follows it. >> >> Why is the client not attempting to recover the open? That's a bug, >> whether or not this patch is correct. > > I added printks in the path of nfs4_do_reclaim(). Client does > "attempt" to recover the open. OPEN never goes on the wire because > can_open_delegated() returned true and no action of adding an rpc task > gets done. Ahh... OK. I'll fix that. Thanks Olga! >> >>> On Fri, Dec 19, 2014 at 3:44 PM, Trond Myklebust >>> <trond.myklebust@primarydata.com> wrote: >>> > Remove an incorrect check for NFS_DELEGATION_NEED_RECLAIM in >>> > can_open_delegated(). We are allowed to cache opens even in >>> > a situation where we're doing reboot recovery. >>> > >>> > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> >>> > --- >>> > fs/nfs/nfs4proc.c | 2 -- >>> > 1 file changed, 2 deletions(-) >>> > >>> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >>> > index 8514b59a8c30..002c7dfedb08 100644 >>> > --- a/fs/nfs/nfs4proc.c >>> > +++ b/fs/nfs/nfs4proc.c >>> > @@ -1119,8 +1119,6 @@ static int can_open_delegated(struct nfs_delegation *delegation, fmode_t fmode) >>> > return 0; >>> > if ((delegation->type & fmode) != fmode) >>> > return 0; >>> > - if (test_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags)) >>> > - return 0; >>> > if (test_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) >>> > return 0; >>> > nfs_mark_delegation_referenced(delegation); >>> > -- >>> > 2.1.0 >>> > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 8514b59a8c30..002c7dfedb08 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1119,8 +1119,6 @@ static int can_open_delegated(struct nfs_delegation *delegation, fmode_t fmode) return 0; if ((delegation->type & fmode) != fmode) return 0; - if (test_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags)) - return 0; if (test_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) return 0; nfs_mark_delegation_referenced(delegation);
Remove an incorrect check for NFS_DELEGATION_NEED_RECLAIM in can_open_delegated(). We are allowed to cache opens even in a situation where we're doing reboot recovery. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/nfs4proc.c | 2 -- 1 file changed, 2 deletions(-)