Messages in this thread | | | Date | Thu, 16 Nov 2023 14:18:49 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH] rxrpc_find_service_conn_rcu: use read_seqbegin() rather than read_seqbegin_or_lock() |
| |
David, Al,
So do you agree that
- the usage of read_seqbegin_or_lock/need_seqretry in this code makes no sense because read_seqlock_excl() is not possible
- this patch doesn't change the current behaviour but simplifies the code and makes it more clear
?
On 10/27, Oleg Nesterov wrote: > > 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@redhat.com> > --- > net/rxrpc/conn_service.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/net/rxrpc/conn_service.c b/net/rxrpc/conn_service.c > index 89ac05a711a4..bfafe58681d9 100644 > --- a/net/rxrpc/conn_service.c > +++ b/net/rxrpc/conn_service.c > @@ -25,7 +25,7 @@ struct rxrpc_connection *rxrpc_find_service_conn_rcu(struct rxrpc_peer *peer, > struct rxrpc_conn_proto k; > struct rxrpc_skb_priv *sp = rxrpc_skb(skb); > struct rb_node *p; > - unsigned int seq = 0; > + unsigned int seq; > > k.epoch = sp->hdr.epoch; > k.cid = sp->hdr.cid & RXRPC_CIDMASK; > @@ -35,7 +35,7 @@ struct rxrpc_connection *rxrpc_find_service_conn_rcu(struct rxrpc_peer *peer, > * under just the RCU read lock, so we have to check for > * changes. > */ > - read_seqbegin_or_lock(&peer->service_conn_lock, &seq); > + seq = read_seqbegin(&peer->service_conn_lock); > > p = rcu_dereference_raw(peer->service_conns.rb_node); > while (p) { > @@ -49,9 +49,8 @@ struct rxrpc_connection *rxrpc_find_service_conn_rcu(struct rxrpc_peer *peer, > break; > conn = NULL; > } > - } while (need_seqretry(&peer->service_conn_lock, seq)); > + } while (read_seqretry(&peer->service_conn_lock, seq)); > > - done_seqretry(&peer->service_conn_lock, seq); > _leave(" = %d", conn ? conn->debug_id : -1); > return conn; > } > -- > 2.25.1.362.g51ebf55 >
| |