* [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
@ 2025-08-11 18:18 Olga Kornievskaia
2025-09-12 14:21 ` Olga Kornievskaia
0 siblings, 1 reply; 14+ messages in thread
From: Olga Kornievskaia @ 2025-08-11 18:18 UTC (permalink / raw)
To: trondmy, anna.schumaker; +Cc: linux-nfs
RFC7530 states that clients should be prepared for the return of
NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
fs/nfs/nfs4proc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 341740fa293d..fa9b81300604 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct file_lock *fl, struct nfs4_state *state,
return err;
do {
err = _nfs4_do_setlk(state, F_SETLK, fl, NFS_LOCK_NEW);
- if (err != -NFS4ERR_DELAY)
+ if (err != -NFS4ERR_DELAY && err != -NFS4ERR_GRACE)
break;
ssleep(1);
- } while (err == -NFS4ERR_DELAY);
+ } while (err == -NFS4ERR_DELAY || err == -NFSERR_GRACE);
return nfs4_handle_delegation_recall_error(server, state, stateid, fl, err);
}
--
2.47.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
2025-08-11 18:18 [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls Olga Kornievskaia
@ 2025-09-12 14:21 ` Olga Kornievskaia
2025-09-12 14:29 ` Trond Myklebust
0 siblings, 1 reply; 14+ messages in thread
From: Olga Kornievskaia @ 2025-09-12 14:21 UTC (permalink / raw)
To: Olga Kornievskaia; +Cc: trondmy, anna.schumaker, linux-nfs
Any comments on or objections to this patch? It does lead to possible
data corruption.
On Mon, Aug 11, 2025 at 2:25 PM Olga Kornievskaia <okorniev@redhat.com> wrote:
>
> RFC7530 states that clients should be prepared for the return of
> NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
>
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> ---
> fs/nfs/nfs4proc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 341740fa293d..fa9b81300604 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct file_lock *fl, struct nfs4_state *state,
> return err;
> do {
> err = _nfs4_do_setlk(state, F_SETLK, fl, NFS_LOCK_NEW);
> - if (err != -NFS4ERR_DELAY)
> + if (err != -NFS4ERR_DELAY && err != -NFS4ERR_GRACE)
> break;
> ssleep(1);
> - } while (err == -NFS4ERR_DELAY);
> + } while (err == -NFS4ERR_DELAY || err == -NFSERR_GRACE);
> return nfs4_handle_delegation_recall_error(server, state, stateid, fl, err);
> }
>
> --
> 2.47.1
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
2025-09-12 14:21 ` Olga Kornievskaia
@ 2025-09-12 14:29 ` Trond Myklebust
2025-09-12 14:41 ` Olga Kornievskaia
0 siblings, 1 reply; 14+ messages in thread
From: Trond Myklebust @ 2025-09-12 14:29 UTC (permalink / raw)
To: Olga Kornievskaia, Olga Kornievskaia; +Cc: anna.schumaker, linux-nfs
On Fri, 2025-09-12 at 10:21 -0400, Olga Kornievskaia wrote:
> Any comments on or objections to this patch? It does lead to possible
> data corruption.
>
Sorry, I think was travelling when you originally sent this patch.
> On Mon, Aug 11, 2025 at 2:25 PM Olga Kornievskaia
> <okorniev@redhat.com> wrote:
> >
> > RFC7530 states that clients should be prepared for the return of
> > NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
> >
> > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > ---
> > fs/nfs/nfs4proc.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 341740fa293d..fa9b81300604 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct
> > file_lock *fl, struct nfs4_state *state,
> > return err;
> > do {
> > err = _nfs4_do_setlk(state, F_SETLK, fl,
> > NFS_LOCK_NEW);
> > - if (err != -NFS4ERR_DELAY)
> > + if (err != -NFS4ERR_DELAY && err != -NFS4ERR_GRACE)
> > break;
> > ssleep(1);
> > - } while (err == -NFS4ERR_DELAY);
> > + } while (err == -NFS4ERR_DELAY || err == -NFSERR_GRACE);
> > return nfs4_handle_delegation_recall_error(server, state,
> > stateid, fl, err);
> > }
> >
> > --
> > 2.47.1
> >
> >
Should the server be sending NFS4ERR_GRACE in this case, though? The
client already holds a delegation, so it is clear that other clients
cannot reclaim any locks that would conflict.
..or is the issue that this could happen before the client has a chance
to reclaim the delegation after a reboot?
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
2025-09-12 14:29 ` Trond Myklebust
@ 2025-09-12 14:41 ` Olga Kornievskaia
2025-09-12 15:11 ` Trond Myklebust
0 siblings, 1 reply; 14+ messages in thread
From: Olga Kornievskaia @ 2025-09-12 14:41 UTC (permalink / raw)
To: Trond Myklebust; +Cc: Olga Kornievskaia, anna.schumaker, linux-nfs
On Fri, Sep 12, 2025 at 10:29 AM Trond Myklebust <trondmy@kernel.org> wrote:
>
> On Fri, 2025-09-12 at 10:21 -0400, Olga Kornievskaia wrote:
> > Any comments on or objections to this patch? It does lead to possible
> > data corruption.
> >
>
> Sorry, I think was travelling when you originally sent this patch.
>
> > On Mon, Aug 11, 2025 at 2:25 PM Olga Kornievskaia
> > <okorniev@redhat.com> wrote:
> > >
> > > RFC7530 states that clients should be prepared for the return of
> > > NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
> > >
> > > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > > ---
> > > fs/nfs/nfs4proc.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > > index 341740fa293d..fa9b81300604 100644
> > > --- a/fs/nfs/nfs4proc.c
> > > +++ b/fs/nfs/nfs4proc.c
> > > @@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct
> > > file_lock *fl, struct nfs4_state *state,
> > > return err;
> > > do {
> > > err = _nfs4_do_setlk(state, F_SETLK, fl,
> > > NFS_LOCK_NEW);
> > > - if (err != -NFS4ERR_DELAY)
> > > + if (err != -NFS4ERR_DELAY && err != -NFS4ERR_GRACE)
> > > break;
> > > ssleep(1);
> > > - } while (err == -NFS4ERR_DELAY);
> > > + } while (err == -NFS4ERR_DELAY || err == -NFSERR_GRACE);
> > > return nfs4_handle_delegation_recall_error(server, state,
> > > stateid, fl, err);
> > > }
> > >
> > > --
> > > 2.47.1
> > >
> > >
>
> Should the server be sending NFS4ERR_GRACE in this case, though? The
> client already holds a delegation, so it is clear that other clients
> cannot reclaim any locks that would conflict.
>
> ..or is the issue that this could happen before the client has a chance
> to reclaim the delegation after a reboot?
The scenario was, v4 client had an open file and a lock and upon
server reboot (during grace) sends the reclaim open, to which the
server replies with a delegation. How a v3 client comes in and
requests the same lock. The linux server at this point sends a
delegation recall to v4 client, the client sends its local lock
request and gets ERR_GRACE.
And the spec explicitly notes as I mention in the commit comment that
the client is supposed to handle ERR_GRACE for non-reclaim locks. Thus
this patch.
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trondmy@kernel.org, trond.myklebust@hammerspace.com
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
2025-09-12 14:41 ` Olga Kornievskaia
@ 2025-09-12 15:11 ` Trond Myklebust
2025-09-12 16:04 ` Olga Kornievskaia
0 siblings, 1 reply; 14+ messages in thread
From: Trond Myklebust @ 2025-09-12 15:11 UTC (permalink / raw)
To: Olga Kornievskaia; +Cc: Olga Kornievskaia, anna.schumaker, linux-nfs
On Fri, 2025-09-12 at 10:41 -0400, Olga Kornievskaia wrote:
> On Fri, Sep 12, 2025 at 10:29 AM Trond Myklebust <trondmy@kernel.org>
> wrote:
> >
> > On Fri, 2025-09-12 at 10:21 -0400, Olga Kornievskaia wrote:
> > > Any comments on or objections to this patch? It does lead to
> > > possible
> > > data corruption.
> > >
> >
> > Sorry, I think was travelling when you originally sent this patch.
> >
> > > On Mon, Aug 11, 2025 at 2:25 PM Olga Kornievskaia
> > > <okorniev@redhat.com> wrote:
> > > >
> > > > RFC7530 states that clients should be prepared for the return
> > > > of
> > > > NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
> > > >
> > > > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > > > ---
> > > > fs/nfs/nfs4proc.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > > > index 341740fa293d..fa9b81300604 100644
> > > > --- a/fs/nfs/nfs4proc.c
> > > > +++ b/fs/nfs/nfs4proc.c
> > > > @@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct
> > > > file_lock *fl, struct nfs4_state *state,
> > > > return err;
> > > > do {
> > > > err = _nfs4_do_setlk(state, F_SETLK, fl,
> > > > NFS_LOCK_NEW);
> > > > - if (err != -NFS4ERR_DELAY)
> > > > + if (err != -NFS4ERR_DELAY && err != -
> > > > NFS4ERR_GRACE)
> > > > break;
> > > > ssleep(1);
> > > > - } while (err == -NFS4ERR_DELAY);
> > > > + } while (err == -NFS4ERR_DELAY || err == -
> > > > NFSERR_GRACE);
> > > > return nfs4_handle_delegation_recall_error(server,
> > > > state,
> > > > stateid, fl, err);
> > > > }
> > > >
> > > > --
> > > > 2.47.1
> > > >
> > > >
> >
> > Should the server be sending NFS4ERR_GRACE in this case, though?
> > The
> > client already holds a delegation, so it is clear that other
> > clients
> > cannot reclaim any locks that would conflict.
> >
> > ..or is the issue that this could happen before the client has a
> > chance
> > to reclaim the delegation after a reboot?
>
To answer my own question here: in that case the server would return
NFS4ERR_BAD_STATEID, and not NFS4ERR_GRACE.
> The scenario was, v4 client had an open file and a lock and upon
> server reboot (during grace) sends the reclaim open, to which the
> server replies with a delegation. How a v3 client comes in and
> requests the same lock. The linux server at this point sends a
> delegation recall to v4 client, the client sends its local lock
> request and gets ERR_GRACE.
>
> And the spec explicitly notes as I mention in the commit comment that
> the client is supposed to handle ERR_GRACE for non-reclaim locks.
> Thus
> this patch.
>
Sure, however the same spec also says (Section 9.6.2.):
If the server can reliably determine that granting a non-reclaim
request will not conflict with reclamation of locks by other clients,
the NFS4ERR_GRACE error does not have to be returned and the
non-reclaim client request can be serviced.
The server can definitely reliably determine that is the case here,
since it already granted the delegation to the client.
I'm not saying that the client shouldn't also handle NFS4ERR_GRACE, but
I am stating that the server shouldn't really be putting us in this
situation in the first place.
I'm also saying that if we're going to handle NFS4ERR_GRACE, then we
also need to handle all the other possible errors under a reboot
scenario.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
2025-09-12 15:11 ` Trond Myklebust
@ 2025-09-12 16:04 ` Olga Kornievskaia
2025-09-29 17:49 ` Olga Kornievskaia
0 siblings, 1 reply; 14+ messages in thread
From: Olga Kornievskaia @ 2025-09-12 16:04 UTC (permalink / raw)
To: Trond Myklebust; +Cc: Olga Kornievskaia, anna.schumaker, linux-nfs
On Fri, Sep 12, 2025 at 11:11 AM Trond Myklebust <trondmy@kernel.org> wrote:
>
> On Fri, 2025-09-12 at 10:41 -0400, Olga Kornievskaia wrote:
> > On Fri, Sep 12, 2025 at 10:29 AM Trond Myklebust <trondmy@kernel.org>
> > wrote:
> > >
> > > On Fri, 2025-09-12 at 10:21 -0400, Olga Kornievskaia wrote:
> > > > Any comments on or objections to this patch? It does lead to
> > > > possible
> > > > data corruption.
> > > >
> > >
> > > Sorry, I think was travelling when you originally sent this patch.
> > >
> > > > On Mon, Aug 11, 2025 at 2:25 PM Olga Kornievskaia
> > > > <okorniev@redhat.com> wrote:
> > > > >
> > > > > RFC7530 states that clients should be prepared for the return
> > > > > of
> > > > > NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
> > > > >
> > > > > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > > > > ---
> > > > > fs/nfs/nfs4proc.c | 4 ++--
> > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > > > > index 341740fa293d..fa9b81300604 100644
> > > > > --- a/fs/nfs/nfs4proc.c
> > > > > +++ b/fs/nfs/nfs4proc.c
> > > > > @@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct
> > > > > file_lock *fl, struct nfs4_state *state,
> > > > > return err;
> > > > > do {
> > > > > err = _nfs4_do_setlk(state, F_SETLK, fl,
> > > > > NFS_LOCK_NEW);
> > > > > - if (err != -NFS4ERR_DELAY)
> > > > > + if (err != -NFS4ERR_DELAY && err != -
> > > > > NFS4ERR_GRACE)
> > > > > break;
> > > > > ssleep(1);
> > > > > - } while (err == -NFS4ERR_DELAY);
> > > > > + } while (err == -NFS4ERR_DELAY || err == -
> > > > > NFSERR_GRACE);
> > > > > return nfs4_handle_delegation_recall_error(server,
> > > > > state,
> > > > > stateid, fl, err);
> > > > > }
> > > > >
> > > > > --
> > > > > 2.47.1
> > > > >
> > > > >
> > >
> > > Should the server be sending NFS4ERR_GRACE in this case, though?
> > > The
> > > client already holds a delegation, so it is clear that other
> > > clients
> > > cannot reclaim any locks that would conflict.
> > >
> > > ..or is the issue that this could happen before the client has a
> > > chance
> > > to reclaim the delegation after a reboot?
> >
> To answer my own question here: in that case the server would return
> NFS4ERR_BAD_STATEID, and not NFS4ERR_GRACE.
>
> > The scenario was, v4 client had an open file and a lock and upon
> > server reboot (during grace) sends the reclaim open, to which the
> > server replies with a delegation. How a v3 client comes in and
> > requests the same lock. The linux server at this point sends a
> > delegation recall to v4 client, the client sends its local lock
> > request and gets ERR_GRACE.
> >
> > And the spec explicitly notes as I mention in the commit comment that
> > the client is supposed to handle ERR_GRACE for non-reclaim locks.
> > Thus
> > this patch.
> >
>
> Sure, however the same spec also says (Section 9.6.2.):
>
> If the server can reliably determine that granting a non-reclaim
> request will not conflict with reclamation of locks by other clients,
> the NFS4ERR_GRACE error does not have to be returned and the
> non-reclaim client request can be serviced.
>
> The server can definitely reliably determine that is the case here,
> since it already granted the delegation to the client.
I'll take your word for it as I'm not that versed in the server code.
But it's an optimization and hard to argue that a server must do it
and thus the client really should handle the case that actually
happens in practice now?
> I'm not saying that the client shouldn't also handle NFS4ERR_GRACE, but
> I am stating that the server shouldn't really be putting us in this
> situation in the first place.
> I'm also saying that if we're going to handle NFS4ERR_GRACE, then we
> also need to handle all the other possible errors under a reboot
> scenario.
I don't see how the "if" and "then" are combined. I think if there are
other errors we don't handle in reclaim then we should but I don't see
it's conditional on handling ERR_GRACE error.
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trondmy@kernel.org, trond.myklebust@hammerspace.com
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
2025-09-12 16:04 ` Olga Kornievskaia
@ 2025-09-29 17:49 ` Olga Kornievskaia
2025-09-30 14:01 ` Chuck Lever
2025-09-30 17:00 ` Olga Kornievskaia
0 siblings, 2 replies; 14+ messages in thread
From: Olga Kornievskaia @ 2025-09-29 17:49 UTC (permalink / raw)
To: Olga Kornievskaia; +Cc: Trond Myklebust, anna.schumaker, linux-nfs
On Fri, Sep 12, 2025 at 12:04 PM Olga Kornievskaia <okorniev@redhat.com> wrote:
>
> On Fri, Sep 12, 2025 at 11:11 AM Trond Myklebust <trondmy@kernel.org> wrote:
> >
> > On Fri, 2025-09-12 at 10:41 -0400, Olga Kornievskaia wrote:
> > > On Fri, Sep 12, 2025 at 10:29 AM Trond Myklebust <trondmy@kernel.org>
> > > wrote:
> > > >
> > > > On Fri, 2025-09-12 at 10:21 -0400, Olga Kornievskaia wrote:
> > > > > Any comments on or objections to this patch? It does lead to
> > > > > possible
> > > > > data corruption.
> > > > >
> > > >
> > > > Sorry, I think was travelling when you originally sent this patch.
> > > >
> > > > > On Mon, Aug 11, 2025 at 2:25 PM Olga Kornievskaia
> > > > > <okorniev@redhat.com> wrote:
> > > > > >
> > > > > > RFC7530 states that clients should be prepared for the return
> > > > > > of
> > > > > > NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
> > > > > >
> > > > > > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > > > > > ---
> > > > > > fs/nfs/nfs4proc.c | 4 ++--
> > > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > > > > > index 341740fa293d..fa9b81300604 100644
> > > > > > --- a/fs/nfs/nfs4proc.c
> > > > > > +++ b/fs/nfs/nfs4proc.c
> > > > > > @@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct
> > > > > > file_lock *fl, struct nfs4_state *state,
> > > > > > return err;
> > > > > > do {
> > > > > > err = _nfs4_do_setlk(state, F_SETLK, fl,
> > > > > > NFS_LOCK_NEW);
> > > > > > - if (err != -NFS4ERR_DELAY)
> > > > > > + if (err != -NFS4ERR_DELAY && err != -
> > > > > > NFS4ERR_GRACE)
> > > > > > break;
> > > > > > ssleep(1);
> > > > > > - } while (err == -NFS4ERR_DELAY);
> > > > > > + } while (err == -NFS4ERR_DELAY || err == -
> > > > > > NFSERR_GRACE);
> > > > > > return nfs4_handle_delegation_recall_error(server,
> > > > > > state,
> > > > > > stateid, fl, err);
> > > > > > }
> > > > > >
> > > > > > --
> > > > > > 2.47.1
> > > > > >
> > > > > >
> > > >
> > > > Should the server be sending NFS4ERR_GRACE in this case, though?
> > > > The
> > > > client already holds a delegation, so it is clear that other
> > > > clients
> > > > cannot reclaim any locks that would conflict.
> > > >
> > > > ..or is the issue that this could happen before the client has a
> > > > chance
> > > > to reclaim the delegation after a reboot?
> > >
> > To answer my own question here: in that case the server would return
> > NFS4ERR_BAD_STATEID, and not NFS4ERR_GRACE.
> >
> > > The scenario was, v4 client had an open file and a lock and upon
> > > server reboot (during grace) sends the reclaim open, to which the
> > > server replies with a delegation. How a v3 client comes in and
> > > requests the same lock. The linux server at this point sends a
> > > delegation recall to v4 client, the client sends its local lock
> > > request and gets ERR_GRACE.
> > >
> > > And the spec explicitly notes as I mention in the commit comment that
> > > the client is supposed to handle ERR_GRACE for non-reclaim locks.
> > > Thus
> > > this patch.
> > >
> >
> > Sure, however the same spec also says (Section 9.6.2.):
> >
> > If the server can reliably determine that granting a non-reclaim
> > request will not conflict with reclamation of locks by other clients,
> > the NFS4ERR_GRACE error does not have to be returned and the
> > non-reclaim client request can be serviced.
> >
> > The server can definitely reliably determine that is the case here,
> > since it already granted the delegation to the client.
>
> I'll take your word for it as I'm not that versed in the server code.
> But it's an optimization and hard to argue that a server must do it
> and thus the client really should handle the case that actually
> happens in practice now?
>
> > I'm not saying that the client shouldn't also handle NFS4ERR_GRACE, but
> > I am stating that the server shouldn't really be putting us in this
> > situation in the first place.
> > I'm also saying that if we're going to handle NFS4ERR_GRACE, then we
> > also need to handle all the other possible errors under a reboot
> > scenario.
>
> I don't see how the "if" and "then" are combined. I think if there are
> other errors we don't handle in reclaim then we should but I don't see
> it's conditional on handling ERR_GRACE error.
What's the path forward here?
>
> > --
> > Trond Myklebust
> > Linux NFS client maintainer, Hammerspace
> > trondmy@kernel.org, trond.myklebust@hammerspace.com
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
2025-09-29 17:49 ` Olga Kornievskaia
@ 2025-09-30 14:01 ` Chuck Lever
2025-09-30 14:29 ` Olga Kornievskaia
2025-09-30 17:00 ` Olga Kornievskaia
1 sibling, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2025-09-30 14:01 UTC (permalink / raw)
To: Olga Kornievskaia, Olga Kornievskaia
Cc: Trond Myklebust, anna.schumaker, linux-nfs
On 9/29/25 1:49 PM, Olga Kornievskaia wrote:
> On Fri, Sep 12, 2025 at 12:04 PM Olga Kornievskaia <okorniev@redhat.com> wrote:
>>
>> On Fri, Sep 12, 2025 at 11:11 AM Trond Myklebust <trondmy@kernel.org> wrote:
>>>
>>> On Fri, 2025-09-12 at 10:41 -0400, Olga Kornievskaia wrote:
>>>> On Fri, Sep 12, 2025 at 10:29 AM Trond Myklebust <trondmy@kernel.org>
>>>> wrote:
>>>>>
>>>>> On Fri, 2025-09-12 at 10:21 -0400, Olga Kornievskaia wrote:
>>>>>> Any comments on or objections to this patch? It does lead to
>>>>>> possible
>>>>>> data corruption.
>>>>>>
>>>>>
>>>>> Sorry, I think was travelling when you originally sent this patch.
>>>>>
>>>>>> On Mon, Aug 11, 2025 at 2:25 PM Olga Kornievskaia
>>>>>> <okorniev@redhat.com> wrote:
>>>>>>>
>>>>>>> RFC7530 states that clients should be prepared for the return
>>>>>>> of
>>>>>>> NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
>>>>>>>
>>>>>>> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
>>>>>>> ---
>>>>>>> fs/nfs/nfs4proc.c | 4 ++--
>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>>>>>> index 341740fa293d..fa9b81300604 100644
>>>>>>> --- a/fs/nfs/nfs4proc.c
>>>>>>> +++ b/fs/nfs/nfs4proc.c
>>>>>>> @@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct
>>>>>>> file_lock *fl, struct nfs4_state *state,
>>>>>>> return err;
>>>>>>> do {
>>>>>>> err = _nfs4_do_setlk(state, F_SETLK, fl,
>>>>>>> NFS_LOCK_NEW);
>>>>>>> - if (err != -NFS4ERR_DELAY)
>>>>>>> + if (err != -NFS4ERR_DELAY && err != -
>>>>>>> NFS4ERR_GRACE)
>>>>>>> break;
>>>>>>> ssleep(1);
>>>>>>> - } while (err == -NFS4ERR_DELAY);
>>>>>>> + } while (err == -NFS4ERR_DELAY || err == -
>>>>>>> NFSERR_GRACE);
>>>>>>> return nfs4_handle_delegation_recall_error(server,
>>>>>>> state,
>>>>>>> stateid, fl, err);
>>>>>>> }
>>>>>>>
>>>>>>> --
>>>>>>> 2.47.1
>>>>>>>
>>>>>>>
>>>>>
>>>>> Should the server be sending NFS4ERR_GRACE in this case, though?
>>>>> The
>>>>> client already holds a delegation, so it is clear that other
>>>>> clients
>>>>> cannot reclaim any locks that would conflict.
>>>>>
>>>>> ..or is the issue that this could happen before the client has a
>>>>> chance
>>>>> to reclaim the delegation after a reboot?
>>>>
>>> To answer my own question here: in that case the server would return
>>> NFS4ERR_BAD_STATEID, and not NFS4ERR_GRACE.
>>>
>>>> The scenario was, v4 client had an open file and a lock and upon
>>>> server reboot (during grace) sends the reclaim open, to which the
>>>> server replies with a delegation. How a v3 client comes in and
>>>> requests the same lock. The linux server at this point sends a
>>>> delegation recall to v4 client, the client sends its local lock
>>>> request and gets ERR_GRACE.
>>>>
>>>> And the spec explicitly notes as I mention in the commit comment that
>>>> the client is supposed to handle ERR_GRACE for non-reclaim locks.
>>>> Thus
>>>> this patch.
>>>>
>>>
>>> Sure, however the same spec also says (Section 9.6.2.):
>>>
>>> If the server can reliably determine that granting a non-reclaim
>>> request will not conflict with reclamation of locks by other clients,
>>> the NFS4ERR_GRACE error does not have to be returned and the
>>> non-reclaim client request can be serviced.
>>>
>>> The server can definitely reliably determine that is the case here,
>>> since it already granted the delegation to the client.
>>
>> I'll take your word for it as I'm not that versed in the server code.
>> But it's an optimization and hard to argue that a server must do it
>> and thus the client really should handle the case that actually
>> happens in practice now?
>>
>>> I'm not saying that the client shouldn't also handle NFS4ERR_GRACE, but
>>> I am stating that the server shouldn't really be putting us in this
>>> situation in the first place.
>>> I'm also saying that if we're going to handle NFS4ERR_GRACE, then we
>>> also need to handle all the other possible errors under a reboot
>>> scenario.
>>
>> I don't see how the "if" and "then" are combined. I think if there are
>> other errors we don't handle in reclaim then we should but I don't see
>> it's conditional on handling ERR_GRACE error.
>
> What's the path forward here?
I saw something earlier in the thread that caught my eye.
It looked like you said that, while NFSD is in grace, it allowed a
client to acquire an NLM lock and that triggered the delegation recall.
It seems to me that, because it was in grace, NFSD should not have
allowed the creation of a new lock; it should have returned nlm_grace.
Did I read that incorrectly?
--
Chuck Lever
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
2025-09-30 14:01 ` Chuck Lever
@ 2025-09-30 14:29 ` Olga Kornievskaia
2025-09-30 14:32 ` Olga Kornievskaia
0 siblings, 1 reply; 14+ messages in thread
From: Olga Kornievskaia @ 2025-09-30 14:29 UTC (permalink / raw)
To: Chuck Lever; +Cc: Olga Kornievskaia, Trond Myklebust, anna.schumaker, linux-nfs
On Tue, Sep 30, 2025 at 10:02 AM Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On 9/29/25 1:49 PM, Olga Kornievskaia wrote:
> > On Fri, Sep 12, 2025 at 12:04 PM Olga Kornievskaia <okorniev@redhat.com> wrote:
> >>
> >> On Fri, Sep 12, 2025 at 11:11 AM Trond Myklebust <trondmy@kernel.org> wrote:
> >>>
> >>> On Fri, 2025-09-12 at 10:41 -0400, Olga Kornievskaia wrote:
> >>>> On Fri, Sep 12, 2025 at 10:29 AM Trond Myklebust <trondmy@kernel.org>
> >>>> wrote:
> >>>>>
> >>>>> On Fri, 2025-09-12 at 10:21 -0400, Olga Kornievskaia wrote:
> >>>>>> Any comments on or objections to this patch? It does lead to
> >>>>>> possible
> >>>>>> data corruption.
> >>>>>>
> >>>>>
> >>>>> Sorry, I think was travelling when you originally sent this patch.
> >>>>>
> >>>>>> On Mon, Aug 11, 2025 at 2:25 PM Olga Kornievskaia
> >>>>>> <okorniev@redhat.com> wrote:
> >>>>>>>
> >>>>>>> RFC7530 states that clients should be prepared for the return
> >>>>>>> of
> >>>>>>> NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
> >>>>>>>
> >>>>>>> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> >>>>>>> ---
> >>>>>>> fs/nfs/nfs4proc.c | 4 ++--
> >>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> >>>>>>> index 341740fa293d..fa9b81300604 100644
> >>>>>>> --- a/fs/nfs/nfs4proc.c
> >>>>>>> +++ b/fs/nfs/nfs4proc.c
> >>>>>>> @@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct
> >>>>>>> file_lock *fl, struct nfs4_state *state,
> >>>>>>> return err;
> >>>>>>> do {
> >>>>>>> err = _nfs4_do_setlk(state, F_SETLK, fl,
> >>>>>>> NFS_LOCK_NEW);
> >>>>>>> - if (err != -NFS4ERR_DELAY)
> >>>>>>> + if (err != -NFS4ERR_DELAY && err != -
> >>>>>>> NFS4ERR_GRACE)
> >>>>>>> break;
> >>>>>>> ssleep(1);
> >>>>>>> - } while (err == -NFS4ERR_DELAY);
> >>>>>>> + } while (err == -NFS4ERR_DELAY || err == -
> >>>>>>> NFSERR_GRACE);
> >>>>>>> return nfs4_handle_delegation_recall_error(server,
> >>>>>>> state,
> >>>>>>> stateid, fl, err);
> >>>>>>> }
> >>>>>>>
> >>>>>>> --
> >>>>>>> 2.47.1
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>> Should the server be sending NFS4ERR_GRACE in this case, though?
> >>>>> The
> >>>>> client already holds a delegation, so it is clear that other
> >>>>> clients
> >>>>> cannot reclaim any locks that would conflict.
> >>>>>
> >>>>> ..or is the issue that this could happen before the client has a
> >>>>> chance
> >>>>> to reclaim the delegation after a reboot?
> >>>>
> >>> To answer my own question here: in that case the server would return
> >>> NFS4ERR_BAD_STATEID, and not NFS4ERR_GRACE.
> >>>
> >>>> The scenario was, v4 client had an open file and a lock and upon
> >>>> server reboot (during grace) sends the reclaim open, to which the
> >>>> server replies with a delegation. How a v3 client comes in and
> >>>> requests the same lock. The linux server at this point sends a
> >>>> delegation recall to v4 client, the client sends its local lock
> >>>> request and gets ERR_GRACE.
> >>>>
> >>>> And the spec explicitly notes as I mention in the commit comment that
> >>>> the client is supposed to handle ERR_GRACE for non-reclaim locks.
> >>>> Thus
> >>>> this patch.
> >>>>
> >>>
> >>> Sure, however the same spec also says (Section 9.6.2.):
> >>>
> >>> If the server can reliably determine that granting a non-reclaim
> >>> request will not conflict with reclamation of locks by other clients,
> >>> the NFS4ERR_GRACE error does not have to be returned and the
> >>> non-reclaim client request can be serviced.
> >>>
> >>> The server can definitely reliably determine that is the case here,
> >>> since it already granted the delegation to the client.
> >>
> >> I'll take your word for it as I'm not that versed in the server code.
> >> But it's an optimization and hard to argue that a server must do it
> >> and thus the client really should handle the case that actually
> >> happens in practice now?
> >>
> >>> I'm not saying that the client shouldn't also handle NFS4ERR_GRACE, but
> >>> I am stating that the server shouldn't really be putting us in this
> >>> situation in the first place.
> >>> I'm also saying that if we're going to handle NFS4ERR_GRACE, then we
> >>> also need to handle all the other possible errors under a reboot
> >>> scenario.
> >>
> >> I don't see how the "if" and "then" are combined. I think if there are
> >> other errors we don't handle in reclaim then we should but I don't see
> >> it's conditional on handling ERR_GRACE error.
> >
> > What's the path forward here?
>
> I saw something earlier in the thread that caught my eye.
>
> It looked like you said that, while NFSD is in grace, it allowed a
> client to acquire an NLM lock and that triggered the delegation recall.
> It seems to me that, because it was in grace, NFSD should not have
> allowed the creation of a new lock; it should have returned nlm_grace.
>
> Did I read that incorrectly?
NFSD did not allow for the creation of a new lock.
NFSD got a v3 lock request which triggered a delegation recall (while
in grace). nfsd v3 call (with the patch a082e4b4d08a "nfsd:
nfserr_jukebox in nlm_fopen should lead to a retry" no longer fails
the request) drops the reply forcing the client to retry. Please
recall that I was advocating for an additional fix where the server
goes a step further and returns nlm_lck_denied_grace_period but it was
not accepted. But that wouldn't have helped the current problem.
Because the delegation is triggered, the client sends a reclaim lock
(but the client already sent a reclaim_complete, as it reclaimed the
open and gotten a delegation) so this is a "new" lock and the server
returns ERR_GRACE. Client does not handle this error and instead acts
like it got the lock and thus silent corruption.
The proposed patch is to handle ERR_GRACE error while reclaiming
delegation state.
>
>
> --
> Chuck Lever
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
2025-09-30 14:29 ` Olga Kornievskaia
@ 2025-09-30 14:32 ` Olga Kornievskaia
2025-09-30 14:37 ` Chuck Lever
0 siblings, 1 reply; 14+ messages in thread
From: Olga Kornievskaia @ 2025-09-30 14:32 UTC (permalink / raw)
To: Chuck Lever; +Cc: Olga Kornievskaia, Trond Myklebust, anna.schumaker, linux-nfs
On Tue, Sep 30, 2025 at 10:29 AM Olga Kornievskaia <aglo@umich.edu> wrote:
>
> On Tue, Sep 30, 2025 at 10:02 AM Chuck Lever <chuck.lever@oracle.com> wrote:
> >
> > On 9/29/25 1:49 PM, Olga Kornievskaia wrote:
> > > On Fri, Sep 12, 2025 at 12:04 PM Olga Kornievskaia <okorniev@redhat.com> wrote:
> > >>
> > >> On Fri, Sep 12, 2025 at 11:11 AM Trond Myklebust <trondmy@kernel.org> wrote:
> > >>>
> > >>> On Fri, 2025-09-12 at 10:41 -0400, Olga Kornievskaia wrote:
> > >>>> On Fri, Sep 12, 2025 at 10:29 AM Trond Myklebust <trondmy@kernel.org>
> > >>>> wrote:
> > >>>>>
> > >>>>> On Fri, 2025-09-12 at 10:21 -0400, Olga Kornievskaia wrote:
> > >>>>>> Any comments on or objections to this patch? It does lead to
> > >>>>>> possible
> > >>>>>> data corruption.
> > >>>>>>
> > >>>>>
> > >>>>> Sorry, I think was travelling when you originally sent this patch.
> > >>>>>
> > >>>>>> On Mon, Aug 11, 2025 at 2:25 PM Olga Kornievskaia
> > >>>>>> <okorniev@redhat.com> wrote:
> > >>>>>>>
> > >>>>>>> RFC7530 states that clients should be prepared for the return
> > >>>>>>> of
> > >>>>>>> NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
> > >>>>>>>
> > >>>>>>> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > >>>>>>> ---
> > >>>>>>> fs/nfs/nfs4proc.c | 4 ++--
> > >>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> > >>>>>>>
> > >>>>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > >>>>>>> index 341740fa293d..fa9b81300604 100644
> > >>>>>>> --- a/fs/nfs/nfs4proc.c
> > >>>>>>> +++ b/fs/nfs/nfs4proc.c
> > >>>>>>> @@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct
> > >>>>>>> file_lock *fl, struct nfs4_state *state,
> > >>>>>>> return err;
> > >>>>>>> do {
> > >>>>>>> err = _nfs4_do_setlk(state, F_SETLK, fl,
> > >>>>>>> NFS_LOCK_NEW);
> > >>>>>>> - if (err != -NFS4ERR_DELAY)
> > >>>>>>> + if (err != -NFS4ERR_DELAY && err != -
> > >>>>>>> NFS4ERR_GRACE)
> > >>>>>>> break;
> > >>>>>>> ssleep(1);
> > >>>>>>> - } while (err == -NFS4ERR_DELAY);
> > >>>>>>> + } while (err == -NFS4ERR_DELAY || err == -
> > >>>>>>> NFSERR_GRACE);
> > >>>>>>> return nfs4_handle_delegation_recall_error(server,
> > >>>>>>> state,
> > >>>>>>> stateid, fl, err);
> > >>>>>>> }
> > >>>>>>>
> > >>>>>>> --
> > >>>>>>> 2.47.1
> > >>>>>>>
> > >>>>>>>
> > >>>>>
> > >>>>> Should the server be sending NFS4ERR_GRACE in this case, though?
> > >>>>> The
> > >>>>> client already holds a delegation, so it is clear that other
> > >>>>> clients
> > >>>>> cannot reclaim any locks that would conflict.
> > >>>>>
> > >>>>> ..or is the issue that this could happen before the client has a
> > >>>>> chance
> > >>>>> to reclaim the delegation after a reboot?
> > >>>>
> > >>> To answer my own question here: in that case the server would return
> > >>> NFS4ERR_BAD_STATEID, and not NFS4ERR_GRACE.
> > >>>
> > >>>> The scenario was, v4 client had an open file and a lock and upon
> > >>>> server reboot (during grace) sends the reclaim open, to which the
> > >>>> server replies with a delegation. How a v3 client comes in and
> > >>>> requests the same lock. The linux server at this point sends a
> > >>>> delegation recall to v4 client, the client sends its local lock
> > >>>> request and gets ERR_GRACE.
> > >>>>
> > >>>> And the spec explicitly notes as I mention in the commit comment that
> > >>>> the client is supposed to handle ERR_GRACE for non-reclaim locks.
> > >>>> Thus
> > >>>> this patch.
> > >>>>
> > >>>
> > >>> Sure, however the same spec also says (Section 9.6.2.):
> > >>>
> > >>> If the server can reliably determine that granting a non-reclaim
> > >>> request will not conflict with reclamation of locks by other clients,
> > >>> the NFS4ERR_GRACE error does not have to be returned and the
> > >>> non-reclaim client request can be serviced.
> > >>>
> > >>> The server can definitely reliably determine that is the case here,
> > >>> since it already granted the delegation to the client.
> > >>
> > >> I'll take your word for it as I'm not that versed in the server code.
> > >> But it's an optimization and hard to argue that a server must do it
> > >> and thus the client really should handle the case that actually
> > >> happens in practice now?
> > >>
> > >>> I'm not saying that the client shouldn't also handle NFS4ERR_GRACE, but
> > >>> I am stating that the server shouldn't really be putting us in this
> > >>> situation in the first place.
> > >>> I'm also saying that if we're going to handle NFS4ERR_GRACE, then we
> > >>> also need to handle all the other possible errors under a reboot
> > >>> scenario.
> > >>
> > >> I don't see how the "if" and "then" are combined. I think if there are
> > >> other errors we don't handle in reclaim then we should but I don't see
> > >> it's conditional on handling ERR_GRACE error.
> > >
> > > What's the path forward here?
> >
> > I saw something earlier in the thread that caught my eye.
> >
> > It looked like you said that, while NFSD is in grace, it allowed a
> > client to acquire an NLM lock and that triggered the delegation recall.
> > It seems to me that, because it was in grace, NFSD should not have
> > allowed the creation of a new lock; it should have returned nlm_grace.
> >
> > Did I read that incorrectly?
>
> NFSD did not allow for the creation of a new lock.
>
> NFSD got a v3 lock request which triggered a delegation recall (while
> in grace). nfsd v3 call (with the patch a082e4b4d08a "nfsd:
> nfserr_jukebox in nlm_fopen should lead to a retry" no longer fails
> the request) drops the reply forcing the client to retry. Please
> recall that I was advocating for an additional fix where the server
> goes a step further and returns nlm_lck_denied_grace_period but it was
> not accepted. But that wouldn't have helped the current problem.
>
> Because the delegation is triggered, the client sends a reclaim lock
> (but the client already sent a reclaim_complete, as it reclaimed the
> open and gotten a delegation) so this is a "new" lock and the server
> returns ERR_GRACE. Client does not handle this error and instead acts
> like it got the lock and thus silent corruption.
>
> The proposed patch is to handle ERR_GRACE error while reclaiming
> delegation state.
>
To clarify there are 2 clients: v3 client (making a new lock request)
and v4 client that holds a delegation (and a local lock).
>
>
> >
> >
> > --
> > Chuck Lever
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
2025-09-30 14:32 ` Olga Kornievskaia
@ 2025-09-30 14:37 ` Chuck Lever
2025-09-30 14:56 ` Olga Kornievskaia
0 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2025-09-30 14:37 UTC (permalink / raw)
To: Olga Kornievskaia
Cc: Olga Kornievskaia, Trond Myklebust, anna.schumaker, linux-nfs
On 9/30/25 10:32 AM, Olga Kornievskaia wrote:
> On Tue, Sep 30, 2025 at 10:29 AM Olga Kornievskaia <aglo@umich.edu> wrote:
>>
>> On Tue, Sep 30, 2025 at 10:02 AM Chuck Lever <chuck.lever@oracle.com> wrote:
>>>
>>> On 9/29/25 1:49 PM, Olga Kornievskaia wrote:
>>>> On Fri, Sep 12, 2025 at 12:04 PM Olga Kornievskaia <okorniev@redhat.com> wrote:
>>>>>
>>>>> On Fri, Sep 12, 2025 at 11:11 AM Trond Myklebust <trondmy@kernel.org> wrote:
>>>>>>
>>>>>> On Fri, 2025-09-12 at 10:41 -0400, Olga Kornievskaia wrote:
>>>>>>> On Fri, Sep 12, 2025 at 10:29 AM Trond Myklebust <trondmy@kernel.org>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> On Fri, 2025-09-12 at 10:21 -0400, Olga Kornievskaia wrote:
>>>>>>>>> Any comments on or objections to this patch? It does lead to
>>>>>>>>> possible
>>>>>>>>> data corruption.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Sorry, I think was travelling when you originally sent this patch.
>>>>>>>>
>>>>>>>>> On Mon, Aug 11, 2025 at 2:25 PM Olga Kornievskaia
>>>>>>>>> <okorniev@redhat.com> wrote:
>>>>>>>>>>
>>>>>>>>>> RFC7530 states that clients should be prepared for the return
>>>>>>>>>> of
>>>>>>>>>> NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
>>>>>>>>>> ---
>>>>>>>>>> fs/nfs/nfs4proc.c | 4 ++--
>>>>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>>>>>>>>> index 341740fa293d..fa9b81300604 100644
>>>>>>>>>> --- a/fs/nfs/nfs4proc.c
>>>>>>>>>> +++ b/fs/nfs/nfs4proc.c
>>>>>>>>>> @@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct
>>>>>>>>>> file_lock *fl, struct nfs4_state *state,
>>>>>>>>>> return err;
>>>>>>>>>> do {
>>>>>>>>>> err = _nfs4_do_setlk(state, F_SETLK, fl,
>>>>>>>>>> NFS_LOCK_NEW);
>>>>>>>>>> - if (err != -NFS4ERR_DELAY)
>>>>>>>>>> + if (err != -NFS4ERR_DELAY && err != -
>>>>>>>>>> NFS4ERR_GRACE)
>>>>>>>>>> break;
>>>>>>>>>> ssleep(1);
>>>>>>>>>> - } while (err == -NFS4ERR_DELAY);
>>>>>>>>>> + } while (err == -NFS4ERR_DELAY || err == -
>>>>>>>>>> NFSERR_GRACE);
>>>>>>>>>> return nfs4_handle_delegation_recall_error(server,
>>>>>>>>>> state,
>>>>>>>>>> stateid, fl, err);
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> 2.47.1
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>> Should the server be sending NFS4ERR_GRACE in this case, though?
>>>>>>>> The
>>>>>>>> client already holds a delegation, so it is clear that other
>>>>>>>> clients
>>>>>>>> cannot reclaim any locks that would conflict.
>>>>>>>>
>>>>>>>> ..or is the issue that this could happen before the client has a
>>>>>>>> chance
>>>>>>>> to reclaim the delegation after a reboot?
>>>>>>>
>>>>>> To answer my own question here: in that case the server would return
>>>>>> NFS4ERR_BAD_STATEID, and not NFS4ERR_GRACE.
>>>>>>
>>>>>>> The scenario was, v4 client had an open file and a lock and upon
>>>>>>> server reboot (during grace) sends the reclaim open, to which the
>>>>>>> server replies with a delegation. How a v3 client comes in and
>>>>>>> requests the same lock. The linux server at this point sends a
>>>>>>> delegation recall to v4 client, the client sends its local lock
>>>>>>> request and gets ERR_GRACE.
>>>>>>>
>>>>>>> And the spec explicitly notes as I mention in the commit comment that
>>>>>>> the client is supposed to handle ERR_GRACE for non-reclaim locks.
>>>>>>> Thus
>>>>>>> this patch.
>>>>>>>
>>>>>>
>>>>>> Sure, however the same spec also says (Section 9.6.2.):
>>>>>>
>>>>>> If the server can reliably determine that granting a non-reclaim
>>>>>> request will not conflict with reclamation of locks by other clients,
>>>>>> the NFS4ERR_GRACE error does not have to be returned and the
>>>>>> non-reclaim client request can be serviced.
>>>>>>
>>>>>> The server can definitely reliably determine that is the case here,
>>>>>> since it already granted the delegation to the client.
>>>>>
>>>>> I'll take your word for it as I'm not that versed in the server code.
>>>>> But it's an optimization and hard to argue that a server must do it
>>>>> and thus the client really should handle the case that actually
>>>>> happens in practice now?
>>>>>
>>>>>> I'm not saying that the client shouldn't also handle NFS4ERR_GRACE, but
>>>>>> I am stating that the server shouldn't really be putting us in this
>>>>>> situation in the first place.
>>>>>> I'm also saying that if we're going to handle NFS4ERR_GRACE, then we
>>>>>> also need to handle all the other possible errors under a reboot
>>>>>> scenario.
>>>>>
>>>>> I don't see how the "if" and "then" are combined. I think if there are
>>>>> other errors we don't handle in reclaim then we should but I don't see
>>>>> it's conditional on handling ERR_GRACE error.
>>>>
>>>> What's the path forward here?
>>>
>>> I saw something earlier in the thread that caught my eye.
>>>
>>> It looked like you said that, while NFSD is in grace, it allowed a
>>> client to acquire an NLM lock and that triggered the delegation recall.
>>> It seems to me that, because it was in grace, NFSD should not have
>>> allowed the creation of a new lock; it should have returned nlm_grace.
>>>
>>> Did I read that incorrectly?
>>
>> NFSD did not allow for the creation of a new lock.
>>
>> NFSD got a v3 lock request which triggered a delegation recall (while
>> in grace). nfsd v3 call (with the patch a082e4b4d08a "nfsd:
>> nfserr_jukebox in nlm_fopen should lead to a retry" no longer fails
>> the request) drops the reply forcing the client to retry. Please
>> recall that I was advocating for an additional fix where the server
>> goes a step further and returns nlm_lck_denied_grace_period but it was
>> not accepted. But that wouldn't have helped the current problem.
>>
>> Because the delegation is triggered, the client sends a reclaim lock
>> (but the client already sent a reclaim_complete, as it reclaimed the
>> open and gotten a delegation) so this is a "new" lock and the server
>> returns ERR_GRACE. Client does not handle this error and instead acts
>> like it got the lock and thus silent corruption.
>>
>> The proposed patch is to handle ERR_GRACE error while reclaiming
>> delegation state.
>>
>
> To clarify there are 2 clients: v3 client (making a new lock request)
> and v4 client that holds a delegation (and a local lock).
I'm still confused about the procession of events. Namely, whether a
delegation recall was done while the server was still in grace.
Could you provide a ladder diagram showing the interactions, in steps?
--
Chuck Lever
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
2025-09-30 14:37 ` Chuck Lever
@ 2025-09-30 14:56 ` Olga Kornievskaia
2025-09-30 15:19 ` Chuck Lever
0 siblings, 1 reply; 14+ messages in thread
From: Olga Kornievskaia @ 2025-09-30 14:56 UTC (permalink / raw)
To: Chuck Lever; +Cc: Olga Kornievskaia, Trond Myklebust, anna.schumaker, linux-nfs
On Tue, Sep 30, 2025 at 10:37 AM Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On 9/30/25 10:32 AM, Olga Kornievskaia wrote:
> > On Tue, Sep 30, 2025 at 10:29 AM Olga Kornievskaia <aglo@umich.edu> wrote:
> >>
> >> On Tue, Sep 30, 2025 at 10:02 AM Chuck Lever <chuck.lever@oracle.com> wrote:
> >>>
> >>> On 9/29/25 1:49 PM, Olga Kornievskaia wrote:
> >>>> On Fri, Sep 12, 2025 at 12:04 PM Olga Kornievskaia <okorniev@redhat.com> wrote:
> >>>>>
> >>>>> On Fri, Sep 12, 2025 at 11:11 AM Trond Myklebust <trondmy@kernel.org> wrote:
> >>>>>>
> >>>>>> On Fri, 2025-09-12 at 10:41 -0400, Olga Kornievskaia wrote:
> >>>>>>> On Fri, Sep 12, 2025 at 10:29 AM Trond Myklebust <trondmy@kernel.org>
> >>>>>>> wrote:
> >>>>>>>>
> >>>>>>>> On Fri, 2025-09-12 at 10:21 -0400, Olga Kornievskaia wrote:
> >>>>>>>>> Any comments on or objections to this patch? It does lead to
> >>>>>>>>> possible
> >>>>>>>>> data corruption.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> Sorry, I think was travelling when you originally sent this patch.
> >>>>>>>>
> >>>>>>>>> On Mon, Aug 11, 2025 at 2:25 PM Olga Kornievskaia
> >>>>>>>>> <okorniev@redhat.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> RFC7530 states that clients should be prepared for the return
> >>>>>>>>>> of
> >>>>>>>>>> NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> >>>>>>>>>> ---
> >>>>>>>>>> fs/nfs/nfs4proc.c | 4 ++--
> >>>>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>>>>>>>
> >>>>>>>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> >>>>>>>>>> index 341740fa293d..fa9b81300604 100644
> >>>>>>>>>> --- a/fs/nfs/nfs4proc.c
> >>>>>>>>>> +++ b/fs/nfs/nfs4proc.c
> >>>>>>>>>> @@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct
> >>>>>>>>>> file_lock *fl, struct nfs4_state *state,
> >>>>>>>>>> return err;
> >>>>>>>>>> do {
> >>>>>>>>>> err = _nfs4_do_setlk(state, F_SETLK, fl,
> >>>>>>>>>> NFS_LOCK_NEW);
> >>>>>>>>>> - if (err != -NFS4ERR_DELAY)
> >>>>>>>>>> + if (err != -NFS4ERR_DELAY && err != -
> >>>>>>>>>> NFS4ERR_GRACE)
> >>>>>>>>>> break;
> >>>>>>>>>> ssleep(1);
> >>>>>>>>>> - } while (err == -NFS4ERR_DELAY);
> >>>>>>>>>> + } while (err == -NFS4ERR_DELAY || err == -
> >>>>>>>>>> NFSERR_GRACE);
> >>>>>>>>>> return nfs4_handle_delegation_recall_error(server,
> >>>>>>>>>> state,
> >>>>>>>>>> stateid, fl, err);
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>> --
> >>>>>>>>>> 2.47.1
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>>> Should the server be sending NFS4ERR_GRACE in this case, though?
> >>>>>>>> The
> >>>>>>>> client already holds a delegation, so it is clear that other
> >>>>>>>> clients
> >>>>>>>> cannot reclaim any locks that would conflict.
> >>>>>>>>
> >>>>>>>> ..or is the issue that this could happen before the client has a
> >>>>>>>> chance
> >>>>>>>> to reclaim the delegation after a reboot?
> >>>>>>>
> >>>>>> To answer my own question here: in that case the server would return
> >>>>>> NFS4ERR_BAD_STATEID, and not NFS4ERR_GRACE.
> >>>>>>
> >>>>>>> The scenario was, v4 client had an open file and a lock and upon
> >>>>>>> server reboot (during grace) sends the reclaim open, to which the
> >>>>>>> server replies with a delegation. How a v3 client comes in and
> >>>>>>> requests the same lock. The linux server at this point sends a
> >>>>>>> delegation recall to v4 client, the client sends its local lock
> >>>>>>> request and gets ERR_GRACE.
> >>>>>>>
> >>>>>>> And the spec explicitly notes as I mention in the commit comment that
> >>>>>>> the client is supposed to handle ERR_GRACE for non-reclaim locks.
> >>>>>>> Thus
> >>>>>>> this patch.
> >>>>>>>
> >>>>>>
> >>>>>> Sure, however the same spec also says (Section 9.6.2.):
> >>>>>>
> >>>>>> If the server can reliably determine that granting a non-reclaim
> >>>>>> request will not conflict with reclamation of locks by other clients,
> >>>>>> the NFS4ERR_GRACE error does not have to be returned and the
> >>>>>> non-reclaim client request can be serviced.
> >>>>>>
> >>>>>> The server can definitely reliably determine that is the case here,
> >>>>>> since it already granted the delegation to the client.
> >>>>>
> >>>>> I'll take your word for it as I'm not that versed in the server code.
> >>>>> But it's an optimization and hard to argue that a server must do it
> >>>>> and thus the client really should handle the case that actually
> >>>>> happens in practice now?
> >>>>>
> >>>>>> I'm not saying that the client shouldn't also handle NFS4ERR_GRACE, but
> >>>>>> I am stating that the server shouldn't really be putting us in this
> >>>>>> situation in the first place.
> >>>>>> I'm also saying that if we're going to handle NFS4ERR_GRACE, then we
> >>>>>> also need to handle all the other possible errors under a reboot
> >>>>>> scenario.
> >>>>>
> >>>>> I don't see how the "if" and "then" are combined. I think if there are
> >>>>> other errors we don't handle in reclaim then we should but I don't see
> >>>>> it's conditional on handling ERR_GRACE error.
> >>>>
> >>>> What's the path forward here?
> >>>
> >>> I saw something earlier in the thread that caught my eye.
> >>>
> >>> It looked like you said that, while NFSD is in grace, it allowed a
> >>> client to acquire an NLM lock and that triggered the delegation recall.
> >>> It seems to me that, because it was in grace, NFSD should not have
> >>> allowed the creation of a new lock; it should have returned nlm_grace.
> >>>
> >>> Did I read that incorrectly?
> >>
> >> NFSD did not allow for the creation of a new lock.
> >>
> >> NFSD got a v3 lock request which triggered a delegation recall (while
> >> in grace). nfsd v3 call (with the patch a082e4b4d08a "nfsd:
> >> nfserr_jukebox in nlm_fopen should lead to a retry" no longer fails
> >> the request) drops the reply forcing the client to retry. Please
> >> recall that I was advocating for an additional fix where the server
> >> goes a step further and returns nlm_lck_denied_grace_period but it was
> >> not accepted. But that wouldn't have helped the current problem.
> >>
> >> Because the delegation is triggered, the client sends a reclaim lock
> >> (but the client already sent a reclaim_complete, as it reclaimed the
> >> open and gotten a delegation) so this is a "new" lock and the server
> >> returns ERR_GRACE. Client does not handle this error and instead acts
> >> like it got the lock and thus silent corruption.
> >>
> >> The proposed patch is to handle ERR_GRACE error while reclaiming
> >> delegation state.
> >>
> >
> > To clarify there are 2 clients: v3 client (making a new lock request)
> > and v4 client that holds a delegation (and a local lock).
>
> I'm still confused about the procession of events. Namely, whether a
> delegation recall was done while the server was still in grace.
Yes. Delegation recall is done while the server is in grace.
> Could you provide a ladder diagram showing the interactions, in steps?
I'm not sure I can do a ladder diagram but let me try to lay out steps.
1. v4 client sends open (foo) and gets a delegation.
2. v4 client locally locks the file.
3. server reboots. grace starts.
4. v4 client sends reclaim open for "foo" and gets a delegation in reply.
5. v4 client sends reclaim_complete
6. v3 client sends NLM lock for "foo".
as a result of step 6 there are 2 actions happen on the server
(a) server sends cb_recall to v4 client
(b) drops NLM lock request for v3 client (so that that the v3 client retries)
7. v4 client sends (non-reclaim) lock request to the server . Server
replies ERR_GRACE.
--> this is where the client doesn't retry but assumes it got the lock.
8. client sends delegreturn.
... if we let this play out. the v3 client which keeps resending NLM
lock request will get the lock once the nfsd is out of grace.
>
>
> --
> Chuck Lever
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
2025-09-30 14:56 ` Olga Kornievskaia
@ 2025-09-30 15:19 ` Chuck Lever
0 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2025-09-30 15:19 UTC (permalink / raw)
To: Olga Kornievskaia
Cc: Olga Kornievskaia, Trond Myklebust, anna.schumaker, linux-nfs
On 9/30/25 10:56 AM, Olga Kornievskaia wrote:
> On Tue, Sep 30, 2025 at 10:37 AM Chuck Lever <chuck.lever@oracle.com> wrote:
>>
>> On 9/30/25 10:32 AM, Olga Kornievskaia wrote:
>>> On Tue, Sep 30, 2025 at 10:29 AM Olga Kornievskaia <aglo@umich.edu> wrote:
>>>>
>>>> On Tue, Sep 30, 2025 at 10:02 AM Chuck Lever <chuck.lever@oracle.com> wrote:
>>>>>
>>>>> On 9/29/25 1:49 PM, Olga Kornievskaia wrote:
>>>>>> On Fri, Sep 12, 2025 at 12:04 PM Olga Kornievskaia <okorniev@redhat.com> wrote:
>>>>>>>
>>>>>>> On Fri, Sep 12, 2025 at 11:11 AM Trond Myklebust <trondmy@kernel.org> wrote:
>>>>>>>>
>>>>>>>> On Fri, 2025-09-12 at 10:41 -0400, Olga Kornievskaia wrote:
>>>>>>>>> On Fri, Sep 12, 2025 at 10:29 AM Trond Myklebust <trondmy@kernel.org>
>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> On Fri, 2025-09-12 at 10:21 -0400, Olga Kornievskaia wrote:
>>>>>>>>>>> Any comments on or objections to this patch? It does lead to
>>>>>>>>>>> possible
>>>>>>>>>>> data corruption.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Sorry, I think was travelling when you originally sent this patch.
>>>>>>>>>>
>>>>>>>>>>> On Mon, Aug 11, 2025 at 2:25 PM Olga Kornievskaia
>>>>>>>>>>> <okorniev@redhat.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> RFC7530 states that clients should be prepared for the return
>>>>>>>>>>>> of
>>>>>>>>>>>> NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
>>>>>>>>>>>> ---
>>>>>>>>>>>> fs/nfs/nfs4proc.c | 4 ++--
>>>>>>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>>>>>>>>>>> index 341740fa293d..fa9b81300604 100644
>>>>>>>>>>>> --- a/fs/nfs/nfs4proc.c
>>>>>>>>>>>> +++ b/fs/nfs/nfs4proc.c
>>>>>>>>>>>> @@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct
>>>>>>>>>>>> file_lock *fl, struct nfs4_state *state,
>>>>>>>>>>>> return err;
>>>>>>>>>>>> do {
>>>>>>>>>>>> err = _nfs4_do_setlk(state, F_SETLK, fl,
>>>>>>>>>>>> NFS_LOCK_NEW);
>>>>>>>>>>>> - if (err != -NFS4ERR_DELAY)
>>>>>>>>>>>> + if (err != -NFS4ERR_DELAY && err != -
>>>>>>>>>>>> NFS4ERR_GRACE)
>>>>>>>>>>>> break;
>>>>>>>>>>>> ssleep(1);
>>>>>>>>>>>> - } while (err == -NFS4ERR_DELAY);
>>>>>>>>>>>> + } while (err == -NFS4ERR_DELAY || err == -
>>>>>>>>>>>> NFSERR_GRACE);
>>>>>>>>>>>> return nfs4_handle_delegation_recall_error(server,
>>>>>>>>>>>> state,
>>>>>>>>>>>> stateid, fl, err);
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> 2.47.1
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Should the server be sending NFS4ERR_GRACE in this case, though?
>>>>>>>>>> The
>>>>>>>>>> client already holds a delegation, so it is clear that other
>>>>>>>>>> clients
>>>>>>>>>> cannot reclaim any locks that would conflict.
>>>>>>>>>>
>>>>>>>>>> ..or is the issue that this could happen before the client has a
>>>>>>>>>> chance
>>>>>>>>>> to reclaim the delegation after a reboot?
>>>>>>>>>
>>>>>>>> To answer my own question here: in that case the server would return
>>>>>>>> NFS4ERR_BAD_STATEID, and not NFS4ERR_GRACE.
>>>>>>>>
>>>>>>>>> The scenario was, v4 client had an open file and a lock and upon
>>>>>>>>> server reboot (during grace) sends the reclaim open, to which the
>>>>>>>>> server replies with a delegation. How a v3 client comes in and
>>>>>>>>> requests the same lock. The linux server at this point sends a
>>>>>>>>> delegation recall to v4 client, the client sends its local lock
>>>>>>>>> request and gets ERR_GRACE.
>>>>>>>>>
>>>>>>>>> And the spec explicitly notes as I mention in the commit comment that
>>>>>>>>> the client is supposed to handle ERR_GRACE for non-reclaim locks.
>>>>>>>>> Thus
>>>>>>>>> this patch.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Sure, however the same spec also says (Section 9.6.2.):
>>>>>>>>
>>>>>>>> If the server can reliably determine that granting a non-reclaim
>>>>>>>> request will not conflict with reclamation of locks by other clients,
>>>>>>>> the NFS4ERR_GRACE error does not have to be returned and the
>>>>>>>> non-reclaim client request can be serviced.
>>>>>>>>
>>>>>>>> The server can definitely reliably determine that is the case here,
>>>>>>>> since it already granted the delegation to the client.
>>>>>>>
>>>>>>> I'll take your word for it as I'm not that versed in the server code.
>>>>>>> But it's an optimization and hard to argue that a server must do it
>>>>>>> and thus the client really should handle the case that actually
>>>>>>> happens in practice now?
>>>>>>>
>>>>>>>> I'm not saying that the client shouldn't also handle NFS4ERR_GRACE, but
>>>>>>>> I am stating that the server shouldn't really be putting us in this
>>>>>>>> situation in the first place.
>>>>>>>> I'm also saying that if we're going to handle NFS4ERR_GRACE, then we
>>>>>>>> also need to handle all the other possible errors under a reboot
>>>>>>>> scenario.
>>>>>>>
>>>>>>> I don't see how the "if" and "then" are combined. I think if there are
>>>>>>> other errors we don't handle in reclaim then we should but I don't see
>>>>>>> it's conditional on handling ERR_GRACE error.
>>>>>>
>>>>>> What's the path forward here?
>>>>>
>>>>> I saw something earlier in the thread that caught my eye.
>>>>>
>>>>> It looked like you said that, while NFSD is in grace, it allowed a
>>>>> client to acquire an NLM lock and that triggered the delegation recall.
>>>>> It seems to me that, because it was in grace, NFSD should not have
>>>>> allowed the creation of a new lock; it should have returned nlm_grace.
>>>>>
>>>>> Did I read that incorrectly?
>>>>
>>>> NFSD did not allow for the creation of a new lock.
>>>>
>>>> NFSD got a v3 lock request which triggered a delegation recall (while
>>>> in grace). nfsd v3 call (with the patch a082e4b4d08a "nfsd:
>>>> nfserr_jukebox in nlm_fopen should lead to a retry" no longer fails
>>>> the request) drops the reply forcing the client to retry. Please
>>>> recall that I was advocating for an additional fix where the server
>>>> goes a step further and returns nlm_lck_denied_grace_period but it was
>>>> not accepted. But that wouldn't have helped the current problem.
>>>>
>>>> Because the delegation is triggered, the client sends a reclaim lock
>>>> (but the client already sent a reclaim_complete, as it reclaimed the
>>>> open and gotten a delegation) so this is a "new" lock and the server
>>>> returns ERR_GRACE. Client does not handle this error and instead acts
>>>> like it got the lock and thus silent corruption.
>>>>
>>>> The proposed patch is to handle ERR_GRACE error while reclaiming
>>>> delegation state.
>>>>
>>>
>>> To clarify there are 2 clients: v3 client (making a new lock request)
>>> and v4 client that holds a delegation (and a local lock).
>>
>> I'm still confused about the procession of events. Namely, whether a
>> delegation recall was done while the server was still in grace.
>
> Yes. Delegation recall is done while the server is in grace.
>
>> Could you provide a ladder diagram showing the interactions, in steps?
>
> I'm not sure I can do a ladder diagram but let me try to lay out steps.
Thanks, this works.
> 1. v4 client sends open (foo) and gets a delegation.
> 2. v4 client locally locks the file.
> 3. server reboots. grace starts.
> 4. v4 client sends reclaim open for "foo" and gets a delegation in reply.
> 5. v4 client sends reclaim_complete
> 6. v3 client sends NLM lock for "foo".
> as a result of step 6 there are 2 actions happen on the server
> (a) server sends cb_recall to v4 client
It's arguable whether the server should prepare to allow the acquisition
of a new lock during its grace period. It might be overall friendlier
behavior if the server did not recall the delegation until grace is
complete for all clients, since it's not supposed to allow new non-
reclaim locks.
I guess it doesn't rise to the category of bug, though.
> (b) drops NLM lock request for v3 client (so that that the v3 client retries)
> 7. v4 client sends (non-reclaim) lock request to the server . Server
> replies ERR_GRACE.
> --> this is where the client doesn't retry but assumes it got the lock.
> 8. client sends delegreturn.
That seems like an overt client bug to me. The client should not return
its delegation until the LOCK request has succeeded.
> ... if we let this play out. the v3 client which keeps resending NLM
> lock request will get the lock once the nfsd is out of grace.
Thank you for clarifying.
--
Chuck Lever
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls
2025-09-29 17:49 ` Olga Kornievskaia
2025-09-30 14:01 ` Chuck Lever
@ 2025-09-30 17:00 ` Olga Kornievskaia
1 sibling, 0 replies; 14+ messages in thread
From: Olga Kornievskaia @ 2025-09-30 17:00 UTC (permalink / raw)
To: Olga Kornievskaia; +Cc: Trond Myklebust, anna.schumaker, linux-nfs
On Mon, Sep 29, 2025 at 1:49 PM Olga Kornievskaia <aglo@umich.edu> wrote:
>
> On Fri, Sep 12, 2025 at 12:04 PM Olga Kornievskaia <okorniev@redhat.com> wrote:
> >
> > On Fri, Sep 12, 2025 at 11:11 AM Trond Myklebust <trondmy@kernel.org> wrote:
> > >
> > > On Fri, 2025-09-12 at 10:41 -0400, Olga Kornievskaia wrote:
> > > > On Fri, Sep 12, 2025 at 10:29 AM Trond Myklebust <trondmy@kernel.org>
> > > > wrote:
> > > > >
> > > > > On Fri, 2025-09-12 at 10:21 -0400, Olga Kornievskaia wrote:
> > > > > > Any comments on or objections to this patch? It does lead to
> > > > > > possible
> > > > > > data corruption.
> > > > > >
> > > > >
> > > > > Sorry, I think was travelling when you originally sent this patch.
> > > > >
> > > > > > On Mon, Aug 11, 2025 at 2:25 PM Olga Kornievskaia
> > > > > > <okorniev@redhat.com> wrote:
> > > > > > >
> > > > > > > RFC7530 states that clients should be prepared for the return
> > > > > > > of
> > > > > > > NFS4ERR_GRACE errors for non-reclaim lock and I/O requests.
> > > > > > >
> > > > > > > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > > > > > > ---
> > > > > > > fs/nfs/nfs4proc.c | 4 ++--
> > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > > > > > > index 341740fa293d..fa9b81300604 100644
> > > > > > > --- a/fs/nfs/nfs4proc.c
> > > > > > > +++ b/fs/nfs/nfs4proc.c
> > > > > > > @@ -7867,10 +7867,10 @@ int nfs4_lock_delegation_recall(struct
> > > > > > > file_lock *fl, struct nfs4_state *state,
> > > > > > > return err;
> > > > > > > do {
> > > > > > > err = _nfs4_do_setlk(state, F_SETLK, fl,
> > > > > > > NFS_LOCK_NEW);
> > > > > > > - if (err != -NFS4ERR_DELAY)
> > > > > > > + if (err != -NFS4ERR_DELAY && err != -
> > > > > > > NFS4ERR_GRACE)
> > > > > > > break;
> > > > > > > ssleep(1);
> > > > > > > - } while (err == -NFS4ERR_DELAY);
> > > > > > > + } while (err == -NFS4ERR_DELAY || err == -
> > > > > > > NFSERR_GRACE);
> > > > > > > return nfs4_handle_delegation_recall_error(server,
> > > > > > > state,
> > > > > > > stateid, fl, err);
> > > > > > > }
> > > > > > >
> > > > > > > --
> > > > > > > 2.47.1
> > > > > > >
> > > > > > >
> > > > >
> > > > > Should the server be sending NFS4ERR_GRACE in this case, though?
> > > > > The
> > > > > client already holds a delegation, so it is clear that other
> > > > > clients
> > > > > cannot reclaim any locks that would conflict.
> > > > >
> > > > > ..or is the issue that this could happen before the client has a
> > > > > chance
> > > > > to reclaim the delegation after a reboot?
> > > >
> > > To answer my own question here: in that case the server would return
> > > NFS4ERR_BAD_STATEID, and not NFS4ERR_GRACE.
> > >
> > > > The scenario was, v4 client had an open file and a lock and upon
> > > > server reboot (during grace) sends the reclaim open, to which the
> > > > server replies with a delegation. How a v3 client comes in and
> > > > requests the same lock. The linux server at this point sends a
> > > > delegation recall to v4 client, the client sends its local lock
> > > > request and gets ERR_GRACE.
> > > >
> > > > And the spec explicitly notes as I mention in the commit comment that
> > > > the client is supposed to handle ERR_GRACE for non-reclaim locks.
> > > > Thus
> > > > this patch.
> > > >
> > >
> > > Sure, however the same spec also says (Section 9.6.2.):
> > >
> > > If the server can reliably determine that granting a non-reclaim
> > > request will not conflict with reclamation of locks by other clients,
> > > the NFS4ERR_GRACE error does not have to be returned and the
> > > non-reclaim client request can be serviced.
> > >
> > > The server can definitely reliably determine that is the case here,
> > > since it already granted the delegation to the client.
> >
> > I'll take your word for it as I'm not that versed in the server code.
> > But it's an optimization and hard to argue that a server must do it
> > and thus the client really should handle the case that actually
> > happens in practice now?
> >
> > > I'm not saying that the client shouldn't also handle NFS4ERR_GRACE, but
> > > I am stating that the server shouldn't really be putting us in this
> > > situation in the first place.
> > > I'm also saying that if we're going to handle NFS4ERR_GRACE, then we
> > > also need to handle all the other possible errors under a reboot
> > > scenario.
> >
> > I don't see how the "if" and "then" are combined. I think if there are
> > other errors we don't handle in reclaim then we should but I don't see
> > it's conditional on handling ERR_GRACE error.
>
> What's the path forward here?
Trond, you mentioned "we also need to handle all the other possible
errors under a reboot scenario".
I could be wrong but I think ERR_GRACE is the only (critical)
unhandled error that's left. _nfs4_do_setlk() via
(nfs4_handle_setlk_error) handles (some) state errors and stale
clientid errors. I think session errors are handled by the
nfs41_sequence_process(). So we are left with ERR_GRACE? Isn't
ERR_GRACE and ERR_DELAY the only 2 errors that get handled by
nfs4_lock_expired and thus we should do the same in reboot recovery
case too?
>
> >
> > > --
> > > Trond Myklebust
> > > Linux NFS client maintainer, Hammerspace
> > > trondmy@kernel.org, trond.myklebust@hammerspace.com
> > >
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-09-30 17:00 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-11 18:18 [PATCH 1/1] NFSv4: handle ERR_GRACE on delegation recalls Olga Kornievskaia
2025-09-12 14:21 ` Olga Kornievskaia
2025-09-12 14:29 ` Trond Myklebust
2025-09-12 14:41 ` Olga Kornievskaia
2025-09-12 15:11 ` Trond Myklebust
2025-09-12 16:04 ` Olga Kornievskaia
2025-09-29 17:49 ` Olga Kornievskaia
2025-09-30 14:01 ` Chuck Lever
2025-09-30 14:29 ` Olga Kornievskaia
2025-09-30 14:32 ` Olga Kornievskaia
2025-09-30 14:37 ` Chuck Lever
2025-09-30 14:56 ` Olga Kornievskaia
2025-09-30 15:19 ` Chuck Lever
2025-09-30 17:00 ` Olga Kornievskaia
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.