* [PATCH 2/3] NFSv4: Fix clearing of layout segments in layoutreturn
2024-08-21 18:05 [PATCH 1/3] NFSv4: Add missing rescheduling points in nfs_client_return_marked_delegations trondmy
@ 2024-08-21 18:05 ` trondmy
2024-08-22 12:07 ` Jeff Layton
2024-08-21 18:05 ` [PATCH 3/3] NFS: Avoid unnecessary rescanning of the per-server delegation list trondmy
2024-08-22 11:58 ` [PATCH 1/3] NFSv4: Add missing rescheduling points in nfs_client_return_marked_delegations Jeff Layton
2 siblings, 1 reply; 6+ messages in thread
From: trondmy @ 2024-08-21 18:05 UTC (permalink / raw)
To: Anna Schumaker; +Cc: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
Make sure that we clear the layout segments in cases where we see a
fatal error, and also in the case where the layout is invalid.
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/nfs4proc.c | 9 ++++++---
fs/nfs/pnfs.c | 5 ++---
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 8883016c551c..daba7d89a0cf 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -9997,6 +9997,7 @@ static void nfs4_layoutreturn_done(struct rpc_task *task, void *calldata)
fallthrough;
default:
task->tk_status = 0;
+ lrp->res.lrs_present = 0;
fallthrough;
case 0:
break;
@@ -10010,9 +10011,11 @@ static void nfs4_layoutreturn_done(struct rpc_task *task, void *calldata)
task->tk_status = 0;
break;
case -NFS4ERR_DELAY:
- if (nfs4_async_handle_error(task, server, NULL, NULL) != -EAGAIN)
- break;
- goto out_restart;
+ if (nfs4_async_handle_error(task, server, NULL, NULL) ==
+ -EAGAIN)
+ goto out_restart;
+ lrp->res.lrs_present = 0;
+ break;
}
return;
out_restart:
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index aa698481bec8..0d16b383a452 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1284,10 +1284,9 @@ void pnfs_layoutreturn_free_lsegs(struct pnfs_layout_hdr *lo,
LIST_HEAD(freeme);
spin_lock(&inode->i_lock);
- if (!pnfs_layout_is_valid(lo) ||
- !nfs4_stateid_match_other(&lo->plh_stateid, arg_stateid))
+ if (!nfs4_stateid_match_other(&lo->plh_stateid, arg_stateid))
goto out_unlock;
- if (stateid) {
+ if (stateid && pnfs_layout_is_valid(lo)) {
u32 seq = be32_to_cpu(arg_stateid->seqid);
pnfs_mark_matching_lsegs_invalid(lo, &freeme, range, seq);
--
2.46.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] NFSv4: Fix clearing of layout segments in layoutreturn
2024-08-21 18:05 ` [PATCH 2/3] NFSv4: Fix clearing of layout segments in layoutreturn trondmy
@ 2024-08-22 12:07 ` Jeff Layton
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2024-08-22 12:07 UTC (permalink / raw)
To: trondmy, Anna Schumaker; +Cc: linux-nfs
On Wed, 2024-08-21 at 14:05 -0400, trondmy@kernel.org wrote:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>
> Make sure that we clear the layout segments in cases where we see a
> fatal error, and also in the case where the layout is invalid.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> fs/nfs/nfs4proc.c | 9 ++++++---
> fs/nfs/pnfs.c | 5 ++---
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 8883016c551c..daba7d89a0cf 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -9997,6 +9997,7 @@ static void nfs4_layoutreturn_done(struct
> rpc_task *task, void *calldata)
> fallthrough;
> default:
> task->tk_status = 0;
> + lrp->res.lrs_present = 0;
> fallthrough;
> case 0:
> break;
> @@ -10010,9 +10011,11 @@ static void nfs4_layoutreturn_done(struct
> rpc_task *task, void *calldata)
> task->tk_status = 0;
> break;
> case -NFS4ERR_DELAY:
> - if (nfs4_async_handle_error(task, server, NULL,
> NULL) != -EAGAIN)
> - break;
> - goto out_restart;
> + if (nfs4_async_handle_error(task, server, NULL,
> NULL) ==
> + -EAGAIN)
> + goto out_restart;
> + lrp->res.lrs_present = 0;
> + break;
> }
> return;
> out_restart:
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index aa698481bec8..0d16b383a452 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -1284,10 +1284,9 @@ void pnfs_layoutreturn_free_lsegs(struct
> pnfs_layout_hdr *lo,
> LIST_HEAD(freeme);
>
> spin_lock(&inode->i_lock);
> - if (!pnfs_layout_is_valid(lo) ||
> - !nfs4_stateid_match_other(&lo->plh_stateid,
> arg_stateid))
> + if (!nfs4_stateid_match_other(&lo->plh_stateid,
> arg_stateid))
> goto out_unlock;
> - if (stateid) {
> + if (stateid && pnfs_layout_is_valid(lo)) {
> u32 seq = be32_to_cpu(arg_stateid->seqid);
>
> pnfs_mark_matching_lsegs_invalid(lo, &freeme, range,
> seq);
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] NFS: Avoid unnecessary rescanning of the per-server delegation list
2024-08-21 18:05 [PATCH 1/3] NFSv4: Add missing rescheduling points in nfs_client_return_marked_delegations trondmy
2024-08-21 18:05 ` [PATCH 2/3] NFSv4: Fix clearing of layout segments in layoutreturn trondmy
@ 2024-08-21 18:05 ` trondmy
2024-08-22 12:19 ` Jeff Layton
2024-08-22 11:58 ` [PATCH 1/3] NFSv4: Add missing rescheduling points in nfs_client_return_marked_delegations Jeff Layton
2 siblings, 1 reply; 6+ messages in thread
From: trondmy @ 2024-08-21 18:05 UTC (permalink / raw)
To: Anna Schumaker; +Cc: linux-nfs
From: Trond Myklebust <trond.myklebust@hammerspace.com>
If the call to nfs_delegation_grab_inode() fails, we will not have
dropped any locks that require us to rescan the list.
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
fs/nfs/delegation.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index d5edb3b3eeef..20cb2008f9e4 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -647,6 +647,9 @@ static int nfs_server_return_marked_delegations(struct nfs_server *server,
prev = delegation;
continue;
}
+ inode = nfs_delegation_grab_inode(delegation);
+ if (inode == NULL)
+ continue;
if (prev) {
struct inode *tmp = nfs_delegation_grab_inode(prev);
@@ -657,12 +660,6 @@ static int nfs_server_return_marked_delegations(struct nfs_server *server,
}
}
- inode = nfs_delegation_grab_inode(delegation);
- if (inode == NULL) {
- rcu_read_unlock();
- iput(to_put);
- goto restart;
- }
delegation = nfs_start_delegation_return_locked(NFS_I(inode));
rcu_read_unlock();
@@ -1184,7 +1181,6 @@ static int nfs_server_reap_unclaimed_delegations(struct nfs_server *server,
struct inode *inode;
restart:
rcu_read_lock();
-restart_locked:
list_for_each_entry_rcu(delegation, &server->delegations, super_list) {
if (test_bit(NFS_DELEGATION_INODE_FREEING,
&delegation->flags) ||
@@ -1195,7 +1191,7 @@ static int nfs_server_reap_unclaimed_delegations(struct nfs_server *server,
continue;
inode = nfs_delegation_grab_inode(delegation);
if (inode == NULL)
- goto restart_locked;
+ continue;
delegation = nfs_start_delegation_return_locked(NFS_I(inode));
rcu_read_unlock();
if (delegation != NULL) {
@@ -1318,7 +1314,6 @@ static int nfs_server_reap_expired_delegations(struct nfs_server *server,
restart:
rcu_read_lock();
-restart_locked:
list_for_each_entry_rcu(delegation, &server->delegations, super_list) {
if (test_bit(NFS_DELEGATION_INODE_FREEING,
&delegation->flags) ||
@@ -1330,7 +1325,7 @@ static int nfs_server_reap_expired_delegations(struct nfs_server *server,
continue;
inode = nfs_delegation_grab_inode(delegation);
if (inode == NULL)
- goto restart_locked;
+ continue;
spin_lock(&delegation->lock);
cred = get_cred_rcu(delegation->cred);
nfs4_stateid_copy(&stateid, &delegation->stateid);
--
2.46.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] NFS: Avoid unnecessary rescanning of the per-server delegation list
2024-08-21 18:05 ` [PATCH 3/3] NFS: Avoid unnecessary rescanning of the per-server delegation list trondmy
@ 2024-08-22 12:19 ` Jeff Layton
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2024-08-22 12:19 UTC (permalink / raw)
To: trondmy, Anna Schumaker; +Cc: linux-nfs
On Wed, 2024-08-21 at 14:05 -0400, trondmy@kernel.org wrote:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>
> If the call to nfs_delegation_grab_inode() fails, we will not have
> dropped any locks that require us to rescan the list.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> fs/nfs/delegation.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
> index d5edb3b3eeef..20cb2008f9e4 100644
> --- a/fs/nfs/delegation.c
> +++ b/fs/nfs/delegation.c
> @@ -647,6 +647,9 @@ static int nfs_server_return_marked_delegations(struct nfs_server *server,
> prev = delegation;
> continue;
> }
> + inode = nfs_delegation_grab_inode(delegation);
> + if (inode == NULL)
> + continue;
>
> if (prev) {
> struct inode *tmp = nfs_delegation_grab_inode(prev);
> @@ -657,12 +660,6 @@ static int nfs_server_return_marked_delegations(struct nfs_server *server,
> }
> }
>
> - inode = nfs_delegation_grab_inode(delegation);
> - if (inode == NULL) {
> - rcu_read_unlock();
> - iput(to_put);
> - goto restart;
> - }
> delegation = nfs_start_delegation_return_locked(NFS_I(inode));
> rcu_read_unlock();
>
> @@ -1184,7 +1181,6 @@ static int nfs_server_reap_unclaimed_delegations(struct nfs_server *server,
> struct inode *inode;
> restart:
> rcu_read_lock();
> -restart_locked:
> list_for_each_entry_rcu(delegation, &server->delegations, super_list) {
> if (test_bit(NFS_DELEGATION_INODE_FREEING,
> &delegation->flags) ||
> @@ -1195,7 +1191,7 @@ static int nfs_server_reap_unclaimed_delegations(struct nfs_server *server,
> continue;
> inode = nfs_delegation_grab_inode(delegation);
> if (inode == NULL)
> - goto restart_locked;
> + continue;
> delegation = nfs_start_delegation_return_locked(NFS_I(inode));
> rcu_read_unlock();
> if (delegation != NULL) {
> @@ -1318,7 +1314,6 @@ static int nfs_server_reap_expired_delegations(struct nfs_server *server,
>
> restart:
> rcu_read_lock();
> -restart_locked:
> list_for_each_entry_rcu(delegation, &server->delegations, super_list) {
> if (test_bit(NFS_DELEGATION_INODE_FREEING,
> &delegation->flags) ||
> @@ -1330,7 +1325,7 @@ static int nfs_server_reap_expired_delegations(struct nfs_server *server,
> continue;
> inode = nfs_delegation_grab_inode(delegation);
> if (inode == NULL)
> - goto restart_locked;
> + continue;
> spin_lock(&delegation->lock);
> cred = get_cred_rcu(delegation->cred);
> nfs4_stateid_copy(&stateid, &delegation->stateid);
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] NFSv4: Add missing rescheduling points in nfs_client_return_marked_delegations
2024-08-21 18:05 [PATCH 1/3] NFSv4: Add missing rescheduling points in nfs_client_return_marked_delegations trondmy
2024-08-21 18:05 ` [PATCH 2/3] NFSv4: Fix clearing of layout segments in layoutreturn trondmy
2024-08-21 18:05 ` [PATCH 3/3] NFS: Avoid unnecessary rescanning of the per-server delegation list trondmy
@ 2024-08-22 11:58 ` Jeff Layton
2 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2024-08-22 11:58 UTC (permalink / raw)
To: trondmy, Anna Schumaker; +Cc: linux-nfs
On Wed, 2024-08-21 at 14:05 -0400, trondmy@kernel.org wrote:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>
> We're seeing reports of soft lockups when iterating through the
> loops,
> so let's add rescheduling points.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> fs/nfs/super.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index cbbd4866b0b7..97b386032b71 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -47,6 +47,7 @@
> #include <linux/vfs.h>
> #include <linux/inet.h>
> #include <linux/in6.h>
> +#include <linux/sched.h>
> #include <linux/slab.h>
> #include <net/ipv6.h>
> #include <linux/netdevice.h>
> @@ -228,6 +229,7 @@ static int __nfs_list_for_each_server(struct
> list_head *head,
> ret = fn(server, data);
> if (ret)
> goto out;
> + cond_resched();
> rcu_read_lock();
> }
> rcu_read_unlock();
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread