linux-kernel - Re: [PATCH v3 3/3] alarmtimer: switch some spin_{lock,unlock}

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6608ee51-0dd9-45d9-ac17-a4e9b736ceb3@nfschina.com>
Date: Wed, 30 Apr 2025 15:19:42 +0800
From: Su Hui <suhui@...china.com>
To: Thomas Gleixner <tglx@...utronix.de>, jstultz@...gle.com, sboyd@...nel.org
Cc: dan.carpenter@...aro.org, linux-kernel@...r.kernel.org,
 kernel-janitors@...r.kernel.org
Subject: Re: [PATCH v3 3/3] alarmtimer: switch some
 spin_{lock,unlock}_irqsave() to guard()

On 2025/4/30 15:04, Thomas Gleixner wrote:
> On Wed, Apr 30 2025 at 11:27, Su Hui wrote:
>> -	spin_lock_irqsave(&rtcdev_lock, flags);
>> -	if (!IS_ERR(pdev) && !rtcdev) {
>> -		if (!try_module_get(rtc->owner)) {
>> +	scoped_guard(spinlock_irqsave, &rtcdev_lock) {
>> +		if (!IS_ERR(pdev) && !rtcdev) {
>> +			if (!try_module_get(rtc->owner)) {
>> +				ret = -1;
>> +				break;
> This really reads odd and relies on the internals of scoped_guard(). I
> actually had to look at the scoped_guard() implementation to validate
> it.
>
> The whole thing can be simplified to:
>
>   	scoped_guard(spinlock_irqsave, &rtcdev_lock) {
> 		if (!IS_ERR(pdev) && !rtcdev && try_module_get(rtc->owner)) {
>   			rtcdev = rtc;
>   			/* hold a reference so it doesn't go away */
>   			get_device(dev);
>                          pdev = NULL;
> 		} else {
>   			ret = -1;
> 		}
>
> No?
Yes, it's simpler and better.
> No need to resend. I fixed it up already.

Thanks a lot :).

Su Hui