s390/time: use assign_bit() where applicable

[PATCH] s390/time: use assign_bit() where applicable

Posted by Peng Fan (OSS) 4 days, 18 hours ago

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

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

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/s390/kernel/time.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index 2b989bebd220..3225a66abfce 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -501,10 +501,7 @@ static int __store_stpinfo(void)
 {
 	int rc = chsc_sstpi(stp_page, &stp_info, sizeof(struct stp_sstpi));
 
-	if (rc)
-		clear_bit(CLOCK_SYNC_STPINFO_VALID, &clock_sync_flags);
-	else
-		set_bit(CLOCK_SYNC_STPINFO_VALID, &clock_sync_flags);
+	assign_bit(CLOCK_SYNC_STPINFO_VALID, &clock_sync_flags, !rc);
 	return rc;
 }
 
@@ -795,10 +792,7 @@ static ssize_t online_store(struct device *dev,
 		return -EOPNOTSUPP;
 	mutex_lock(&stp_mutex);
 	stp_online = value;
-	if (stp_online)
-		set_bit(CLOCK_SYNC_STP, &clock_sync_flags);
-	else
-		clear_bit(CLOCK_SYNC_STP, &clock_sync_flags);
+	assign_bit(CLOCK_SYNC_STP, &clock_sync_flags, stp_online);
 	queue_work(time_sync_wq, &stp_work);
 	mutex_unlock(&stp_mutex);
 	return count;
-- 
2.51.0

Re: [PATCH] s390/time: use assign_bit() where applicable

Posted by Heiko Carstens 4 days, 4 hours ago

On Sun, Sep 20, 2026 at 10:28:00AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Convert open-coded if/else with set_bit/clear_bit to the assign_bit API.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  arch/s390/kernel/time.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
...
> -	if (rc)
> -		clear_bit(CLOCK_SYNC_STPINFO_VALID, &clock_sync_flags);
> -	else
> -		set_bit(CLOCK_SYNC_STPINFO_VALID, &clock_sync_flags);
> +	assign_bit(CLOCK_SYNC_STPINFO_VALID, &clock_sync_flags, !rc);
>  	return rc;
...
>  	mutex_lock(&stp_mutex);
>  	stp_online = value;
> -	if (stp_online)
> -		set_bit(CLOCK_SYNC_STP, &clock_sync_flags);
> -	else
> -		clear_bit(CLOCK_SYNC_STP, &clock_sync_flags);
> +	assign_bit(CLOCK_SYNC_STP, &clock_sync_flags, stp_online);

I don't know why all those trivial helper functions which obfuscate
the code are introduced. Before it was very obvious what the code did,
now I have to look up assign_bit() just to figure out that it is a
completely trivial helper function, with close to zero benefit.

Re: [PATCH] s390/time: use assign_bit() where applicable

Posted by Peng Fan 3 days, 19 hours ago

On Sun, Sep 20, 2026 at 06:22:16PM +0200, Heiko Carstens wrote:
>On Sun, Sep 20, 2026 at 10:28:00AM +0800, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@nxp.com>
>> 
>> Convert open-coded if/else with set_bit/clear_bit to the assign_bit API.
>> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> ---
>>  arch/s390/kernel/time.c | 10 ++--------
>>  1 file changed, 2 insertions(+), 8 deletions(-)
>...
>> -	if (rc)
>> -		clear_bit(CLOCK_SYNC_STPINFO_VALID, &clock_sync_flags);
>> -	else
>> -		set_bit(CLOCK_SYNC_STPINFO_VALID, &clock_sync_flags);
>> +	assign_bit(CLOCK_SYNC_STPINFO_VALID, &clock_sync_flags, !rc);
>>  	return rc;
>...
>>  	mutex_lock(&stp_mutex);
>>  	stp_online = value;
>> -	if (stp_online)
>> -		set_bit(CLOCK_SYNC_STP, &clock_sync_flags);
>> -	else
>> -		clear_bit(CLOCK_SYNC_STP, &clock_sync_flags);
>> +	assign_bit(CLOCK_SYNC_STP, &clock_sync_flags, stp_online);
>
>I don't know why all those trivial helper functions which obfuscate
>the code are introduced. Before it was very obvious what the code did,
>now I have to look up assign_bit() just to figure out that it is a
>completely trivial helper function, with close to zero benefit.

This API was introduced by
9a8ac3ae682e ("dm mpath: cleanup QUEUE_IF_NO_PATH bit manipulation by introducing assign_bit()")

And moved to include/linux/bitops.h for broader usage.

I think it would be good for us to save lines.

Drop this patch since you disable it.

Thanks,
Peng
>
>