linux-kernel - [PATCH] fs/afs: use read_seqbegin() rather than read_seqbegin_or

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:   Fri, 27 Oct 2023 12:12:05 +0200
From:   Oleg Nesterov <oleg@...hat.com>
To:     David Howells <dhowells@...hat.com>,
        Marc Dionne <marc.dionne@...istor.com>
Cc:     Chuck Lever <chuck.lever@...cle.com>,
        linux-afs@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: [PATCH] fs/afs: use read_seqbegin() rather than
 read_seqbegin_or_lock()

read_seqbegin_or_lock() makes no sense unless you make "seq" odd
after the lockless access failed. See thread_group_cputime() as
an example, note that it does nextseq = 1 for the 2nd round.

So this code can use read_seqbegin() without changing the current
behaviour.

Signed-off-by: Oleg Nesterov <oleg@...hat.com>
---
 fs/afs/callback.c |  7 +++----
 fs/afs/inode.c    | 15 ++++++---------
 fs/afs/server.c   | 17 +++++++----------
 3 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/fs/afs/callback.c b/fs/afs/callback.c
index a484fa642808..b33d8a4901f8 100644
--- a/fs/afs/callback.c
+++ b/fs/afs/callback.c
@@ -110,14 +110,14 @@ static struct afs_volume *afs_lookup_volume_rcu(struct afs_cell *cell,
 {
 	struct afs_volume *volume = NULL;
 	struct rb_node *p;
-	int seq = 0;
+	int seq;
 
 	do {
 		/* Unfortunately, rbtree walking doesn't give reliable results
 		 * under just the RCU read lock, so we have to check for
 		 * changes.
 		 */
-		read_seqbegin_or_lock(&cell->volume_lock, &seq);
+		seq = read_seqbegin(&cell->volume_lock);
 
 		p = rcu_dereference_raw(cell->volumes.rb_node);
 		while (p) {
@@ -132,9 +132,8 @@ static struct afs_volume *afs_lookup_volume_rcu(struct afs_cell *cell,
 			volume = NULL;
 		}
 
-	} while (need_seqretry(&cell->volume_lock, seq));
+	} while (read_seqretry(&cell->volume_lock, seq));
 
-	done_seqretry(&cell->volume_lock, seq);
 	return volume;
 }
 
diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 1c794a1896aa..88003fefff71 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -629,10 +629,10 @@ bool afs_check_validity(struct afs_vnode *vnode)
 	enum afs_cb_break_reason need_clear = afs_cb_break_no_break;
 	time64_t now = ktime_get_real_seconds();
 	unsigned int cb_break;
-	int seq = 0;
+	int seq;
 
 	do {
-		read_seqbegin_or_lock(&vnode->cb_lock, &seq);
+		seq = read_seqbegin(&vnode->cb_lock);
 		cb_break = vnode->cb_break;
 
 		if (test_bit(AFS_VNODE_CB_PROMISED, &vnode->flags)) {
@@ -650,9 +650,7 @@ bool afs_check_validity(struct afs_vnode *vnode)
 			need_clear = afs_cb_break_no_promise;
 		}
 
-	} while (need_seqretry(&vnode->cb_lock, seq));
-
-	done_seqretry(&vnode->cb_lock, seq);
+	} while (read_seqretry(&vnode->cb_lock, seq));
 
 	if (need_clear == afs_cb_break_no_break)
 		return true;
@@ -755,7 +753,7 @@ int afs_getattr(struct mnt_idmap *idmap, const struct path *path,
 	struct inode *inode = d_inode(path->dentry);
 	struct afs_vnode *vnode = AFS_FS_I(inode);
 	struct key *key;
-	int ret, seq = 0;
+	int ret, seq;
 
 	_enter("{ ino=%lu v=%u }", inode->i_ino, inode->i_generation);
 
@@ -772,7 +770,7 @@ int afs_getattr(struct mnt_idmap *idmap, const struct path *path,
 	}
 
 	do {
-		read_seqbegin_or_lock(&vnode->cb_lock, &seq);
+		seq = read_seqbegin(&vnode->cb_lock);
 		generic_fillattr(&nop_mnt_idmap, request_mask, inode, stat);
 		if (test_bit(AFS_VNODE_SILLY_DELETED, &vnode->flags) &&
 		    stat->nlink > 0)
@@ -784,9 +782,8 @@ int afs_getattr(struct mnt_idmap *idmap, const struct path *path,
 		 */
 		if (S_ISDIR(inode->i_mode))
 			stat->size = vnode->netfs.remote_i_size;
-	} while (need_seqretry(&vnode->cb_lock, seq));
+	} while (read_seqretry(&vnode->cb_lock, seq));
 
-	done_seqretry(&vnode->cb_lock, seq);
 	return 0;
 }
 
diff --git a/fs/afs/server.c b/fs/afs/server.c
index b5237206eac3..01a26c0e0d28 100644
--- a/fs/afs/server.c
+++ b/fs/afs/server.c
@@ -27,7 +27,7 @@ struct afs_server *afs_find_server(struct afs_net *net,
 	const struct afs_addr_list *alist;
 	struct afs_server *server = NULL;
 	unsigned int i;
-	int seq = 0, diff;
+	int seq, diff;
 
 	rcu_read_lock();
 
@@ -35,7 +35,8 @@ struct afs_server *afs_find_server(struct afs_net *net,
 		if (server)
 			afs_unuse_server_notime(net, server, afs_server_trace_put_find_rsq);
 		server = NULL;
-		read_seqbegin_or_lock(&net->fs_addr_lock, &seq);
+
+		seq = read_seqbegin(&net->fs_addr_lock);
 
 		if (srx->transport.family == AF_INET6) {
 			const struct sockaddr_in6 *a = &srx->transport.sin6, *b;
@@ -75,9 +76,7 @@ struct afs_server *afs_find_server(struct afs_net *net,
 	found:
 		server = afs_maybe_use_server(server, afs_server_trace_get_by_addr);
 
-	} while (need_seqretry(&net->fs_addr_lock, seq));
-
-	done_seqretry(&net->fs_addr_lock, seq);
+	} while (read_seqretry(&net->fs_addr_lock, seq));
 
 	rcu_read_unlock();
 	return server;
@@ -90,7 +89,7 @@ struct afs_server *afs_find_server_by_uuid(struct afs_net *net, const uuid_t *uu
 {
 	struct afs_server *server = NULL;
 	struct rb_node *p;
-	int diff, seq = 0;
+	int diff, seq;
 
 	_enter("%pU", uuid);
 
@@ -103,7 +102,7 @@ struct afs_server *afs_find_server_by_uuid(struct afs_net *net, const uuid_t *uu
 			afs_unuse_server(net, server, afs_server_trace_put_uuid_rsq);
 		server = NULL;
 
-		read_seqbegin_or_lock(&net->fs_lock, &seq);
+		seq = read_seqbegin(&net->fs_lock);
 
 		p = net->fs_servers.rb_node;
 		while (p) {
@@ -121,9 +120,7 @@ struct afs_server *afs_find_server_by_uuid(struct afs_net *net, const uuid_t *uu
 
 			server = NULL;
 		}
-	} while (need_seqretry(&net->fs_lock, seq));
-
-	done_seqretry(&net->fs_lock, seq);
+	} while (read_seqretry(&net->fs_lock, seq));
 
 	_leave(" = %p", server);
 	return server;
-- 
2.25.1.362.g51ebf55