[PATCH] SUNRPC: use assign_bit() where applicable

[PATCH] SUNRPC: use assign_bit() where applicable

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Convert open-coded if/else with set_bit/clear_bit the assign_bit API.

Done with Coccinelle semantic patch:
    // set_bit -> clear_bit => assign_bit

    @@
    expression cond, bit, addr;
    @@

    -if (cond)
    -        set_bit(bit, addr);
    -else
    -        clear_bit(bit, addr);
    +assign_bit(bit, addr, cond);

    @@
    expression cond, bit, addr;
    @@

    -if (cond)
    -        clear_bit(bit, addr);
    -else
    -        set_bit(bit, addr);
    +assign_bit(bit, addr, !cond);

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 net/sunrpc/svcsock.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index ef7ac080fcd3..d7fa0d1de3ef 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -352,10 +352,8 @@ static void svc_sock_setbufsize(struct svc_sock *svsk, unsigned int nreqs)
 
 static void svc_sock_secure_port(struct svc_rqst *rqstp)
 {
-	if (svc_port_is_privileged(svc_addr(rqstp)))
-		set_bit(RQ_SECURE, &rqstp->rq_flags);
-	else
-		clear_bit(RQ_SECURE, &rqstp->rq_flags);
+	assign_bit(RQ_SECURE, &rqstp->rq_flags,
+		   svc_port_is_privileged(svc_addr(rqstp)));
 }
 
 /*
@@ -941,10 +939,8 @@ static struct svc_xprt *svc_tcp_accept(struct svc_xprt *xprt)
 		slen = offsetof(struct sockaddr, sa_data);
 	svc_xprt_set_local(&newsvsk->sk_xprt, sin, slen);
 
-	if (sock_is_loopback(newsock->sk))
-		set_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags);
-	else
-		clear_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags);
+	assign_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags,
+		   sock_is_loopback(newsock->sk));
 	if (serv->sv_stats)
 		serv->sv_stats->nettcpconn++;
 
@@ -1290,10 +1286,8 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp)
 
 	rqstp->rq_xprt_ctxt   = NULL;
 	rqstp->rq_prot	      = IPPROTO_TCP;
-	if (test_bit(XPT_LOCAL, &svsk->sk_xprt.xpt_flags))
-		set_bit(RQ_LOCAL, &rqstp->rq_flags);
-	else
-		clear_bit(RQ_LOCAL, &rqstp->rq_flags);
+	assign_bit(RQ_LOCAL, &rqstp->rq_flags,
+		   test_bit(XPT_LOCAL, &svsk->sk_xprt.xpt_flags));
 
 	/* Completing one message stops ->read_sock with whatever
 	 * follows still queued, and no path from here re-arms XPT_DATA.
-- 
2.51.0

Re: [PATCH] SUNRPC: use assign_bit() where applicable

[Chuck Lever]

On Sat, Sep 19, 2026, at 10:27 PM, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Convert open-coded if/else with set_bit/clear_bit the assign_bit API.

The above sentence explains the same thing that the diff body shows me,
so it does not add value.

But I don't have any context here: why is this being done? Is there some
kind of tree-wide clean-up underway so that a new feature can be added,
or is this patch just a one-off change?

Including a URL that points to an explainer, or making this patch part
of a series would help orient reviewers.


> Done with Coccinelle semantic patch:
>     // set_bit -> clear_bit => assign_bit
>
>     @@
>     expression cond, bit, addr;
>     @@
>
>     -if (cond)
>     -        set_bit(bit, addr);
>     -else
>     -        clear_bit(bit, addr);
>     +assign_bit(bit, addr, cond);
>
>     @@
>     expression cond, bit, addr;
>     @@
>
>     -if (cond)
>     -        clear_bit(bit, addr);
>     -else
>     -        set_bit(bit, addr);
>     +assign_bit(bit, addr, !cond);
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  net/sunrpc/svcsock.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index ef7ac080fcd3..d7fa0d1de3ef 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -352,10 +352,8 @@ static void svc_sock_setbufsize(struct svc_sock 
> *svsk, unsigned int nreqs)
> 
>  static void svc_sock_secure_port(struct svc_rqst *rqstp)
>  {
> -	if (svc_port_is_privileged(svc_addr(rqstp)))
> -		set_bit(RQ_SECURE, &rqstp->rq_flags);
> -	else
> -		clear_bit(RQ_SECURE, &rqstp->rq_flags);
> +	assign_bit(RQ_SECURE, &rqstp->rq_flags,
> +		   svc_port_is_privileged(svc_addr(rqstp)));
>  }
> 
>  /*
> @@ -941,10 +939,8 @@ static struct svc_xprt *svc_tcp_accept(struct 
> svc_xprt *xprt)
>  		slen = offsetof(struct sockaddr, sa_data);
>  	svc_xprt_set_local(&newsvsk->sk_xprt, sin, slen);
> 
> -	if (sock_is_loopback(newsock->sk))
> -		set_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags);
> -	else
> -		clear_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags);
> +	assign_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags,
> +		   sock_is_loopback(newsock->sk));
>  	if (serv->sv_stats)
>  		serv->sv_stats->nettcpconn++;
> 
> @@ -1290,10 +1286,8 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp)
> 
>  	rqstp->rq_xprt_ctxt   = NULL;
>  	rqstp->rq_prot	      = IPPROTO_TCP;
> -	if (test_bit(XPT_LOCAL, &svsk->sk_xprt.xpt_flags))
> -		set_bit(RQ_LOCAL, &rqstp->rq_flags);
> -	else
> -		clear_bit(RQ_LOCAL, &rqstp->rq_flags);
> +	assign_bit(RQ_LOCAL, &rqstp->rq_flags,
> +		   test_bit(XPT_LOCAL, &svsk->sk_xprt.xpt_flags));
> 
>  	/* Completing one message stops ->read_sock with whatever
>  	 * follows still queued, and no path from here re-arms XPT_DATA.
> -- 
> 2.51.0

-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

Re: [PATCH] SUNRPC: use assign_bit() where applicable

[Peng Fan]

On Sun, Sep 20, 2026 at 01:22:00PM -0400, Chuck Lever wrote:
>
>
>On Sat, Sep 19, 2026, at 10:27 PM, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@nxp.com>
>>
>> Convert open-coded if/else with set_bit/clear_bit the assign_bit API.
>
>The above sentence explains the same thing that the diff body shows me,
>so it does not add value.
>
>But I don't have any context here: why is this being done? Is there some
>kind of tree-wide clean-up underway so that a new feature can be added,
>or is this patch just a one-off change?

It is just a one-off change. I don't group other patches to use assign_bit()
into a large patchset, just separate patches.

Free to drop if it does not make sense to you.

Thanks,
Peng

>
>Including a URL that points to an explainer, or making this patch part
>of a series would help orient reviewers.
>
>
>> Done with Coccinelle semantic patch:
>>     // set_bit -> clear_bit => assign_bit
>>
>>     @@
>>     expression cond, bit, addr;
>>     @@
>>
>>     -if (cond)
>>     -        set_bit(bit, addr);
>>     -else
>>     -        clear_bit(bit, addr);
>>     +assign_bit(bit, addr, cond);
>>
>>     @@
>>     expression cond, bit, addr;
>>     @@
>>
>>     -if (cond)
>>     -        clear_bit(bit, addr);
>>     -else
>>     -        set_bit(bit, addr);
>>     +assign_bit(bit, addr, !cond);
>>
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> ---
>>  net/sunrpc/svcsock.c | 18 ++++++------------
>>  1 file changed, 6 insertions(+), 12 deletions(-)
>>
>> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
>> index ef7ac080fcd3..d7fa0d1de3ef 100644
>> --- a/net/sunrpc/svcsock.c
>> +++ b/net/sunrpc/svcsock.c
>> @@ -352,10 +352,8 @@ static void svc_sock_setbufsize(struct svc_sock 
>> *svsk, unsigned int nreqs)
>> 
>>  static void svc_sock_secure_port(struct svc_rqst *rqstp)
>>  {
>> -	if (svc_port_is_privileged(svc_addr(rqstp)))
>> -		set_bit(RQ_SECURE, &rqstp->rq_flags);
>> -	else
>> -		clear_bit(RQ_SECURE, &rqstp->rq_flags);
>> +	assign_bit(RQ_SECURE, &rqstp->rq_flags,
>> +		   svc_port_is_privileged(svc_addr(rqstp)));
>>  }
>> 
>>  /*
>> @@ -941,10 +939,8 @@ static struct svc_xprt *svc_tcp_accept(struct 
>> svc_xprt *xprt)
>>  		slen = offsetof(struct sockaddr, sa_data);
>>  	svc_xprt_set_local(&newsvsk->sk_xprt, sin, slen);
>> 
>> -	if (sock_is_loopback(newsock->sk))
>> -		set_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags);
>> -	else
>> -		clear_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags);
>> +	assign_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags,
>> +		   sock_is_loopback(newsock->sk));
>>  	if (serv->sv_stats)
>>  		serv->sv_stats->nettcpconn++;
>> 
>> @@ -1290,10 +1286,8 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp)
>> 
>>  	rqstp->rq_xprt_ctxt   = NULL;
>>  	rqstp->rq_prot	      = IPPROTO_TCP;
>> -	if (test_bit(XPT_LOCAL, &svsk->sk_xprt.xpt_flags))
>> -		set_bit(RQ_LOCAL, &rqstp->rq_flags);
>> -	else
>> -		clear_bit(RQ_LOCAL, &rqstp->rq_flags);
>> +	assign_bit(RQ_LOCAL, &rqstp->rq_flags,
>> +		   test_bit(XPT_LOCAL, &svsk->sk_xprt.xpt_flags));
>> 
>>  	/* Completing one message stops ->read_sock with whatever
>>  	 * follows still queued, and no path from here re-arms XPT_DATA.
>> -- 
>> 2.51.0
>
>-- 
>Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)
>
>

Re: [PATCH] SUNRPC: use assign_bit() where applicable

[Chuck Lever]

On Sun, Sep 20, 2026, at 8:51 PM, Peng Fan wrote:
> On Sun, Sep 20, 2026 at 01:22:00PM -0400, Chuck Lever wrote:
>>
>>
>>On Sat, Sep 19, 2026, at 10:27 PM, Peng Fan (OSS) wrote:
>>> From: Peng Fan <peng.fan@nxp.com>
>>>
>>> Convert open-coded if/else with set_bit/clear_bit the assign_bit API.
>>
>>The above sentence explains the same thing that the diff body shows me,
>>so it does not add value.
>>
>>But I don't have any context here: why is this being done? Is there some
>>kind of tree-wide clean-up underway so that a new feature can be added,
>>or is this patch just a one-off change?
>
> It is just a one-off change. I don't group other patches to use assign_bit()
> into a large patchset, just separate patches.
>
> Free to drop if it does not make sense to you.

It makes sense. But I'd rather not make assumptions.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

Re: [PATCH] SUNRPC: use assign_bit() where applicable

[Jeff Layton]

On Mon, 2026-09-21 at 08:51 +0800, Peng Fan wrote:
> On Sun, Sep 20, 2026 at 01:22:00PM -0400, Chuck Lever wrote:
> > 
> > 
> > On Sat, Sep 19, 2026, at 10:27 PM, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > > 
> > > Convert open-coded if/else with set_bit/clear_bit the assign_bit API.
> > 
> > The above sentence explains the same thing that the diff body shows me,
> > so it does not add value.
> > 
> > But I don't have any context here: why is this being done? Is there some
> > kind of tree-wide clean-up underway so that a new feature can be added,
> > or is this patch just a one-off change?
> 
> It is just a one-off change. I don't group other patches to use assign_bit()
> into a large patchset, just separate patches.
> 
> Free to drop if it does not make sense to you.
> 
> 

I think the problem is that you haven't explained the benefit of using
assign_bit() here. It's now 2 lines instead of 4, but that doesn't seem
like a substantive change. Does this create better assembly or
something? 

Without a clear benefit, this seems like pointless churn.

> > Including a URL that points to an explainer, or making this patch part
> > of a series would help orient reviewers.
> > 
> > 
> > > Done with Coccinelle semantic patch:
> > >     // set_bit -> clear_bit => assign_bit
> > > 
> > >     @@
> > >     expression cond, bit, addr;
> > >     @@
> > > 
> > >     -if (cond)
> > >     -        set_bit(bit, addr);
> > >     -else
> > >     -        clear_bit(bit, addr);
> > >     +assign_bit(bit, addr, cond);
> > > 
> > >     @@
> > >     expression cond, bit, addr;
> > >     @@
> > > 
> > >     -if (cond)
> > >     -        clear_bit(bit, addr);
> > >     -else
> > >     -        set_bit(bit, addr);
> > >     +assign_bit(bit, addr, !cond);
> > > 
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  net/sunrpc/svcsock.c | 18 ++++++------------
> > >  1 file changed, 6 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> > > index ef7ac080fcd3..d7fa0d1de3ef 100644
> > > --- a/net/sunrpc/svcsock.c
> > > +++ b/net/sunrpc/svcsock.c
> > > @@ -352,10 +352,8 @@ static void svc_sock_setbufsize(struct svc_sock 
> > > *svsk, unsigned int nreqs)
> > > 
> > >  static void svc_sock_secure_port(struct svc_rqst *rqstp)
> > >  {
> > > -	if (svc_port_is_privileged(svc_addr(rqstp)))
> > > -		set_bit(RQ_SECURE, &rqstp->rq_flags);
> > > -	else
> > > -		clear_bit(RQ_SECURE, &rqstp->rq_flags);
> > > +	assign_bit(RQ_SECURE, &rqstp->rq_flags,
> > > +		   svc_port_is_privileged(svc_addr(rqstp)));
> > >  }
> > > 
> > >  /*
> > > @@ -941,10 +939,8 @@ static struct svc_xprt *svc_tcp_accept(struct 
> > > svc_xprt *xprt)
> > >  		slen = offsetof(struct sockaddr, sa_data);
> > >  	svc_xprt_set_local(&newsvsk->sk_xprt, sin, slen);
> > > 
> > > -	if (sock_is_loopback(newsock->sk))
> > > -		set_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags);
> > > -	else
> > > -		clear_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags);
> > > +	assign_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags,
> > > +		   sock_is_loopback(newsock->sk));
> > >  	if (serv->sv_stats)
> > >  		serv->sv_stats->nettcpconn++;
> > > 
> > > @@ -1290,10 +1286,8 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp)
> > > 
> > >  	rqstp->rq_xprt_ctxt   = NULL;
> > >  	rqstp->rq_prot	      = IPPROTO_TCP;
> > > -	if (test_bit(XPT_LOCAL, &svsk->sk_xprt.xpt_flags))
> > > -		set_bit(RQ_LOCAL, &rqstp->rq_flags);
> > > -	else
> > > -		clear_bit(RQ_LOCAL, &rqstp->rq_flags);
> > > +	assign_bit(RQ_LOCAL, &rqstp->rq_flags,
> > > +		   test_bit(XPT_LOCAL, &svsk->sk_xprt.xpt_flags));
> > > 
> > >  	/* Completing one message stops ->read_sock with whatever
> > >  	 * follows still queued, and no path from here re-arms XPT_DATA.
> > > -- 
> > > 2.51.0
> > 
> > -- 
> > Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)
> > 
> > 

-- 
Jeff Layton <jlayton@kernel.org>