linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] NFSv4: Add missing rescheduling points in nfs_client_return_marked_delegations
@ 2024-08-21 18:05 trondmy
  2024-08-21 18:05 ` [PATCH 2/3] NFSv4: Fix clearing of layout segments in layoutreturn trondmy
                   ` (2 more replies)
  0 siblings, 3 replies; 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>

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();
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [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

* [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 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

* 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

* 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

end of thread, other threads:[~2024-08-22 12:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-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 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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).