Re: [PATCH v3 3/3] alarmtimer: switch some spin_{lock,unlock}_irqsave() to guard()

[Thomas Gleixner <tglx@xxxxxxxxxxxxx>]

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?

No need to resend. I fixed it up already.

Thanks,

        tglx