mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/3] time: some cleanup for jiffies and alarmtimer
@ 2025-04-30  3:27 Su Hui
  2025-04-30  3:27 ` [PATCH v3 1/3] time/jiffies: change register_refined_jiffies() to void __init Su Hui
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Su Hui @ 2025-04-30  3:27 UTC (permalink / raw)
  To: jstultz, tglx, sboyd
  Cc: Su Hui, eahariha, luiz.von.dentz, anna-maria, ojeda,
	linux-kernel, kernel-janitors

There are some small cleanup for jiffies.c and alarmtimer.c.
Compile test only.

v3:
 - add a WARN_ON_ONCE in patch 2.

v2:
 - remove some guard() useages in patch 3.
 - https://lore.kernel.org/all/20250427113529.1473800-1-suhui@nfschina.com/

v1:
 - https://lore.kernel.org/all/20250424144819.24884-1-suhui@nfschina.com/


Su Hui (3):
  time/jiffies: change register_refined_jiffies() to void __init
  alarmtimer: remove dead return value in clock2alarm()
  alarmtimer: switch some spin_{lock,unlock}_irqsave() to guard()

 include/linux/jiffies.h  |  2 +-
 kernel/time/alarmtimer.c | 62 +++++++++++++++++-----------------------
 kernel/time/jiffies.c    |  5 +---
 3 files changed, 28 insertions(+), 41 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/3] time/jiffies: change register_refined_jiffies() to void __init
  2025-04-30  3:27 [PATCH v3 0/3] time: some cleanup for jiffies and alarmtimer Su Hui
@ 2025-04-30  3:27 ` Su Hui
  2025-04-30  7:22   ` [tip: timers/core] time/jiffies: Change " tip-bot2 for Su Hui
  2025-04-30  3:27 ` [PATCH v3 2/3] alarmtimer: remove dead return value in clock2alarm() Su Hui
  2025-04-30  3:27 ` [PATCH v3 3/3] alarmtimer: switch some spin_{lock,unlock}_irqsave() to guard() Su Hui
  2 siblings, 1 reply; 10+ messages in thread
From: Su Hui @ 2025-04-30  3:27 UTC (permalink / raw)
  To: jstultz
  Cc: Su Hui, tglx, eahariha, geert, anna-maria, ojeda, linux-kernel,
	kernel-janitors

register_refined_jiffies() is only used in setup code and always return 0.
Mark it to __init to save some bytes and change it to void.

Signed-off-by: Su Hui <suhui@nfschina.com>
---
v2/v3:
 - No change. 

 include/linux/jiffies.h | 2 +-
 kernel/time/jiffies.c   | 5 +----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
index 0ea8c9887429..91b20788273d 100644
--- a/include/linux/jiffies.h
+++ b/include/linux/jiffies.h
@@ -59,7 +59,7 @@
 /* LATCH is used in the interval timer and ftape setup. */
 #define LATCH ((CLOCK_TICK_RATE + HZ/2) / HZ)	/* For divider */
 
-extern int register_refined_jiffies(long clock_tick_rate);
+extern void register_refined_jiffies(long clock_tick_rate);
 
 /* TICK_USEC is the time between ticks in usec assuming SHIFTED_HZ */
 #define TICK_USEC ((USEC_PER_SEC + HZ/2) / HZ)
diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
index bc4db9e5ab70..34eeacac2253 100644
--- a/kernel/time/jiffies.c
+++ b/kernel/time/jiffies.c
@@ -75,13 +75,11 @@ struct clocksource * __init __weak clocksource_default_clock(void)
 
 static struct clocksource refined_jiffies;
 
-int register_refined_jiffies(long cycles_per_second)
+void __init register_refined_jiffies(long cycles_per_second)
 {
 	u64 nsec_per_tick, shift_hz;
 	long cycles_per_tick;
 
-
-
 	refined_jiffies = clocksource_jiffies;
 	refined_jiffies.name = "refined-jiffies";
 	refined_jiffies.rating++;
@@ -100,5 +98,4 @@ int register_refined_jiffies(long cycles_per_second)
 	refined_jiffies.mult = ((u32)nsec_per_tick) << JIFFIES_SHIFT;
 
 	__clocksource_register(&refined_jiffies);
-	return 0;
 }
-- 
2.30.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 2/3] alarmtimer: remove dead return value in clock2alarm()
  2025-04-30  3:27 [PATCH v3 0/3] time: some cleanup for jiffies and alarmtimer Su Hui
  2025-04-30  3:27 ` [PATCH v3 1/3] time/jiffies: change register_refined_jiffies() to void __init Su Hui
@ 2025-04-30  3:27 ` Su Hui
  2025-04-30  7:22   ` [tip: timers/core] alarmtimer: Remove " tip-bot2 for Su Hui
  2025-04-30 22:14   ` [PATCH v3 2/3] alarmtimer: remove " John Stultz
  2025-04-30  3:27 ` [PATCH v3 3/3] alarmtimer: switch some spin_{lock,unlock}_irqsave() to guard() Su Hui
  2 siblings, 2 replies; 10+ messages in thread
From: Su Hui @ 2025-04-30  3:27 UTC (permalink / raw)
  To: jstultz, tglx, sboyd; +Cc: Su Hui, linux-kernel, kernel-janitors

'clockid' only can be ALARM_REALTIME and ALARM_BOOTTIME. It's impossible
to return -1 and callers never check the value of -1.

Only alarm_clock_get_timespec(), alarm_clock_get_ktime(),
alarm_timer_create() and alarm_timer_nsleep() call clock2alarm(). These
callers using clockid_to_kclock() to get 'struct k_clock', this ensures
clock2alarm() never returns -1.

Remove the impossible -1 return value, and add a warn to be aware of any
misuse of this function.

Signed-off-by: Su Hui <suhui@nfschina.com>
---
v3:
 - Add WARN_ON_ONCE to notice any misuse of this function.

v2:
 - No Change.

 kernel/time/alarmtimer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 0ddccdff119a..621d39666e29 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -515,9 +515,9 @@ static enum alarmtimer_type clock2alarm(clockid_t clockid)
 {
 	if (clockid == CLOCK_REALTIME_ALARM)
 		return ALARM_REALTIME;
-	if (clockid == CLOCK_BOOTTIME_ALARM)
-		return ALARM_BOOTTIME;
-	return -1;
+
+	WARN_ON_ONCE(clockid != CLOCK_BOOTTIME_ALARM);
+	return ALARM_BOOTTIME;
 }
 
 /**
-- 
2.30.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 3/3] alarmtimer: switch some spin_{lock,unlock}_irqsave() to guard()
  2025-04-30  3:27 [PATCH v3 0/3] time: some cleanup for jiffies and alarmtimer Su Hui
  2025-04-30  3:27 ` [PATCH v3 1/3] time/jiffies: change register_refined_jiffies() to void __init Su Hui
  2025-04-30  3:27 ` [PATCH v3 2/3] alarmtimer: remove dead return value in clock2alarm() Su Hui
@ 2025-04-30  3:27 ` Su Hui
  2025-04-30  7:04   ` Thomas Gleixner
  2025-04-30  7:22   ` [tip: timers/core] alarmtimer: Switch spin_{lock,unlock}_irqsave() to guards tip-bot2 for Su Hui
  2 siblings, 2 replies; 10+ messages in thread
From: Su Hui @ 2025-04-30  3:27 UTC (permalink / raw)
  To: jstultz, tglx, sboyd; +Cc: Su Hui, dan.carpenter, linux-kernel, kernel-janitors

Using guard/scoped_guard() to simplify code. Using guard() to remove
'goto unlock' label is neater especially.

Signed-off-by: Su Hui <suhui@nfschina.com>
Acked-by: John Stultz <jstultz@google.com>
---
v3:
 - No Change.

v2:
 - Remove some guard() usages acrodding to john's suggestions.

 kernel/time/alarmtimer.c | 56 +++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 33 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 621d39666e29..9f75aaf1ba35 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -70,12 +70,10 @@ static DEFINE_SPINLOCK(rtcdev_lock);
  */
 struct rtc_device *alarmtimer_get_rtcdev(void)
 {
-	unsigned long flags;
 	struct rtc_device *ret;
 
-	spin_lock_irqsave(&rtcdev_lock, flags);
+	guard(spinlock_irqsave)(&rtcdev_lock);
 	ret = rtcdev;
-	spin_unlock_irqrestore(&rtcdev_lock, flags);
 
 	return ret;
 }
@@ -83,7 +81,6 @@ EXPORT_SYMBOL_GPL(alarmtimer_get_rtcdev);
 
 static int alarmtimer_rtc_add_device(struct device *dev)
 {
-	unsigned long flags;
 	struct rtc_device *rtc = to_rtc_device(dev);
 	struct platform_device *pdev;
 	int ret = 0;
@@ -101,22 +98,21 @@ static int alarmtimer_rtc_add_device(struct device *dev)
 	if (!IS_ERR(pdev))
 		device_init_wakeup(&pdev->dev, true);
 
-	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;
+			}
+
+			rtcdev = rtc;
+			/* hold a reference so it doesn't go away */
+			get_device(dev);
+			pdev = NULL;
+		} else {
 			ret = -1;
-			goto unlock;
 		}
-
-		rtcdev = rtc;
-		/* hold a reference so it doesn't go away */
-		get_device(dev);
-		pdev = NULL;
-	} else {
-		ret = -1;
 	}
-unlock:
-	spin_unlock_irqrestore(&rtcdev_lock, flags);
 
 	platform_device_unregister(pdev);
 
@@ -198,7 +194,7 @@ static enum hrtimer_restart alarmtimer_fired(struct hrtimer *timer)
 	struct alarm *alarm = container_of(timer, struct alarm, timer);
 	struct alarm_base *base = &alarm_bases[alarm->type];
 
-	scoped_guard (spinlock_irqsave, &base->lock)
+	scoped_guard(spinlock_irqsave, &base->lock)
 		alarmtimer_dequeue(base, alarm);
 
 	if (alarm->function)
@@ -251,9 +247,8 @@ static int alarmtimer_suspend(struct device *dev)
 		struct timerqueue_node *next;
 		ktime_t delta;
 
-		spin_lock_irqsave(&base->lock, flags);
-		next = timerqueue_getnext(&base->timerqueue);
-		spin_unlock_irqrestore(&base->lock, flags);
+		scoped_guard(spinlock_irqsave, &base->lock)
+			next = timerqueue_getnext(&base->timerqueue);
 		if (!next)
 			continue;
 		delta = ktime_sub(next->expires, base->get_ktime());
@@ -381,13 +376,11 @@ EXPORT_SYMBOL_GPL(alarm_start_relative);
 void alarm_restart(struct alarm *alarm)
 {
 	struct alarm_base *base = &alarm_bases[alarm->type];
-	unsigned long flags;
 
-	spin_lock_irqsave(&base->lock, flags);
+	guard(spinlock_irqsave)(&base->lock);
 	hrtimer_set_expires(&alarm->timer, alarm->node.expires);
 	hrtimer_restart(&alarm->timer);
 	alarmtimer_enqueue(base, alarm);
-	spin_unlock_irqrestore(&base->lock, flags);
 }
 EXPORT_SYMBOL_GPL(alarm_restart);
 
@@ -401,14 +394,13 @@ EXPORT_SYMBOL_GPL(alarm_restart);
 int alarm_try_to_cancel(struct alarm *alarm)
 {
 	struct alarm_base *base = &alarm_bases[alarm->type];
-	unsigned long flags;
 	int ret;
 
-	spin_lock_irqsave(&base->lock, flags);
-	ret = hrtimer_try_to_cancel(&alarm->timer);
-	if (ret >= 0)
-		alarmtimer_dequeue(base, alarm);
-	spin_unlock_irqrestore(&base->lock, flags);
+	scoped_guard(spinlock_irqsave, &base->lock) {
+		ret = hrtimer_try_to_cancel(&alarm->timer);
+		if (ret >= 0)
+			alarmtimer_dequeue(base, alarm);
+	}
 
 	trace_alarmtimer_cancel(alarm, base->get_ktime());
 	return ret;
@@ -479,7 +471,6 @@ EXPORT_SYMBOL_GPL(alarm_forward_now);
 static void alarmtimer_freezerset(ktime_t absexp, enum alarmtimer_type type)
 {
 	struct alarm_base *base;
-	unsigned long flags;
 	ktime_t delta;
 
 	switch(type) {
@@ -498,13 +489,12 @@ static void alarmtimer_freezerset(ktime_t absexp, enum alarmtimer_type type)
 
 	delta = ktime_sub(absexp, base->get_ktime());
 
-	spin_lock_irqsave(&freezer_delta_lock, flags);
+	guard(spinlock_irqsave)(&freezer_delta_lock);
 	if (!freezer_delta || (delta < freezer_delta)) {
 		freezer_delta = delta;
 		freezer_expires = absexp;
 		freezer_alarmtype = type;
 	}
-	spin_unlock_irqrestore(&freezer_delta_lock, flags);
 }
 
 /**
-- 
2.30.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/3] alarmtimer: switch some spin_{lock,unlock}_irqsave() to guard()
  2025-04-30  3:27 ` [PATCH v3 3/3] alarmtimer: switch some spin_{lock,unlock}_irqsave() to guard() Su Hui
@ 2025-04-30  7:04   ` Thomas Gleixner
  2025-04-30  7:19     ` Su Hui
  2025-04-30  7:22   ` [tip: timers/core] alarmtimer: Switch spin_{lock,unlock}_irqsave() to guards tip-bot2 for Su Hui
  1 sibling, 1 reply; 10+ messages in thread
From: Thomas Gleixner @ 2025-04-30  7:04 UTC (permalink / raw)
  To: Su Hui, jstultz, sboyd
  Cc: Su Hui, dan.carpenter, linux-kernel, kernel-janitors

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/3] alarmtimer: switch some spin_{lock,unlock}_irqsave() to guard()
  2025-04-30  7:04   ` Thomas Gleixner
@ 2025-04-30  7:19     ` Su Hui
  0 siblings, 0 replies; 10+ messages in thread
From: Su Hui @ 2025-04-30  7:19 UTC (permalink / raw)
  To: Thomas Gleixner, jstultz, sboyd
  Cc: dan.carpenter, linux-kernel, kernel-janitors

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tip: timers/core] alarmtimer: Switch spin_{lock,unlock}_irqsave() to guards
  2025-04-30  3:27 ` [PATCH v3 3/3] alarmtimer: switch some spin_{lock,unlock}_irqsave() to guard() Su Hui
  2025-04-30  7:04   ` Thomas Gleixner
@ 2025-04-30  7:22   ` tip-bot2 for Su Hui
  1 sibling, 0 replies; 10+ messages in thread
From: tip-bot2 for Su Hui @ 2025-04-30  7:22 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Su Hui, Thomas Gleixner, John Stultz, x86, linux-kernel

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     2117c1d503b4e0fd0c6776ae9fe4df2260643eae
Gitweb:        https://git.kernel.org/tip/2117c1d503b4e0fd0c6776ae9fe4df2260643eae
Author:        Su Hui <suhui@nfschina.com>
AuthorDate:    Wed, 30 Apr 2025 11:27:34 +08:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 30 Apr 2025 09:06:23 +02:00

alarmtimer: Switch spin_{lock,unlock}_irqsave() to guards

Using guard/scoped_guard() to simplify code. Using guard() to remove
'goto unlock' label is neater especially.

[ tglx: Brought back the scoped_guard()'s which were dropped in v2 and
  	simplified alarmtimer_rtc_add_device() ]

Signed-off-by: Su Hui <suhui@nfschina.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: John Stultz <jstultz@google.com>
Link: https://lore.kernel.org/all/20250430032734.2079290-4-suhui@nfschina.com

---
 kernel/time/alarmtimer.c | 78 +++++++++++++++------------------------
 1 file changed, 30 insertions(+), 48 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 621d396..577f0e6 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -70,12 +70,10 @@ static DEFINE_SPINLOCK(rtcdev_lock);
  */
 struct rtc_device *alarmtimer_get_rtcdev(void)
 {
-	unsigned long flags;
 	struct rtc_device *ret;
 
-	spin_lock_irqsave(&rtcdev_lock, flags);
+	guard(spinlock_irqsave)(&rtcdev_lock);
 	ret = rtcdev;
-	spin_unlock_irqrestore(&rtcdev_lock, flags);
 
 	return ret;
 }
@@ -83,7 +81,6 @@ EXPORT_SYMBOL_GPL(alarmtimer_get_rtcdev);
 
 static int alarmtimer_rtc_add_device(struct device *dev)
 {
-	unsigned long flags;
 	struct rtc_device *rtc = to_rtc_device(dev);
 	struct platform_device *pdev;
 	int ret = 0;
@@ -101,25 +98,18 @@ static int alarmtimer_rtc_add_device(struct device *dev)
 	if (!IS_ERR(pdev))
 		device_init_wakeup(&pdev->dev, true);
 
-	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 && try_module_get(rtc->owner)) {
+			rtcdev = rtc;
+			/* hold a reference so it doesn't go away */
+			get_device(dev);
+			pdev = NULL;
+		} else {
 			ret = -1;
-			goto unlock;
 		}
-
-		rtcdev = rtc;
-		/* hold a reference so it doesn't go away */
-		get_device(dev);
-		pdev = NULL;
-	} else {
-		ret = -1;
 	}
-unlock:
-	spin_unlock_irqrestore(&rtcdev_lock, flags);
 
 	platform_device_unregister(pdev);
-
 	return ret;
 }
 
@@ -198,7 +188,7 @@ static enum hrtimer_restart alarmtimer_fired(struct hrtimer *timer)
 	struct alarm *alarm = container_of(timer, struct alarm, timer);
 	struct alarm_base *base = &alarm_bases[alarm->type];
 
-	scoped_guard (spinlock_irqsave, &base->lock)
+	scoped_guard(spinlock_irqsave, &base->lock)
 		alarmtimer_dequeue(base, alarm);
 
 	if (alarm->function)
@@ -228,17 +218,16 @@ EXPORT_SYMBOL_GPL(alarm_expires_remaining);
 static int alarmtimer_suspend(struct device *dev)
 {
 	ktime_t min, now, expires;
-	int i, ret, type;
 	struct rtc_device *rtc;
-	unsigned long flags;
 	struct rtc_time tm;
+	int i, ret, type;
 
-	spin_lock_irqsave(&freezer_delta_lock, flags);
-	min = freezer_delta;
-	expires = freezer_expires;
-	type = freezer_alarmtype;
-	freezer_delta = 0;
-	spin_unlock_irqrestore(&freezer_delta_lock, flags);
+	scoped_guard(spinlock_irqsave, &freezer_delta_lock) {
+		min = freezer_delta;
+		expires = freezer_expires;
+		type = freezer_alarmtype;
+		freezer_delta = 0;
+	}
 
 	rtc = alarmtimer_get_rtcdev();
 	/* If we have no rtcdev, just return */
@@ -251,9 +240,8 @@ static int alarmtimer_suspend(struct device *dev)
 		struct timerqueue_node *next;
 		ktime_t delta;
 
-		spin_lock_irqsave(&base->lock, flags);
-		next = timerqueue_getnext(&base->timerqueue);
-		spin_unlock_irqrestore(&base->lock, flags);
+		scoped_guard(spinlock_irqsave, &base->lock)
+			next = timerqueue_getnext(&base->timerqueue);
 		if (!next)
 			continue;
 		delta = ktime_sub(next->expires, base->get_ktime());
@@ -352,13 +340,12 @@ EXPORT_SYMBOL_GPL(alarm_init);
 void alarm_start(struct alarm *alarm, ktime_t start)
 {
 	struct alarm_base *base = &alarm_bases[alarm->type];
-	unsigned long flags;
 
-	spin_lock_irqsave(&base->lock, flags);
-	alarm->node.expires = start;
-	alarmtimer_enqueue(base, alarm);
-	hrtimer_start(&alarm->timer, alarm->node.expires, HRTIMER_MODE_ABS);
-	spin_unlock_irqrestore(&base->lock, flags);
+	scoped_guard(spinlock_irqsave, &base->lock) {
+		alarm->node.expires = start;
+		alarmtimer_enqueue(base, alarm);
+		hrtimer_start(&alarm->timer, alarm->node.expires, HRTIMER_MODE_ABS);
+	}
 
 	trace_alarmtimer_start(alarm, base->get_ktime());
 }
@@ -381,13 +368,11 @@ EXPORT_SYMBOL_GPL(alarm_start_relative);
 void alarm_restart(struct alarm *alarm)
 {
 	struct alarm_base *base = &alarm_bases[alarm->type];
-	unsigned long flags;
 
-	spin_lock_irqsave(&base->lock, flags);
+	guard(spinlock_irqsave)(&base->lock);
 	hrtimer_set_expires(&alarm->timer, alarm->node.expires);
 	hrtimer_restart(&alarm->timer);
 	alarmtimer_enqueue(base, alarm);
-	spin_unlock_irqrestore(&base->lock, flags);
 }
 EXPORT_SYMBOL_GPL(alarm_restart);
 
@@ -401,14 +386,13 @@ EXPORT_SYMBOL_GPL(alarm_restart);
 int alarm_try_to_cancel(struct alarm *alarm)
 {
 	struct alarm_base *base = &alarm_bases[alarm->type];
-	unsigned long flags;
 	int ret;
 
-	spin_lock_irqsave(&base->lock, flags);
-	ret = hrtimer_try_to_cancel(&alarm->timer);
-	if (ret >= 0)
-		alarmtimer_dequeue(base, alarm);
-	spin_unlock_irqrestore(&base->lock, flags);
+	scoped_guard(spinlock_irqsave, &base->lock) {
+		ret = hrtimer_try_to_cancel(&alarm->timer);
+		if (ret >= 0)
+			alarmtimer_dequeue(base, alarm);
+	}
 
 	trace_alarmtimer_cancel(alarm, base->get_ktime());
 	return ret;
@@ -479,7 +463,6 @@ EXPORT_SYMBOL_GPL(alarm_forward_now);
 static void alarmtimer_freezerset(ktime_t absexp, enum alarmtimer_type type)
 {
 	struct alarm_base *base;
-	unsigned long flags;
 	ktime_t delta;
 
 	switch(type) {
@@ -498,13 +481,12 @@ static void alarmtimer_freezerset(ktime_t absexp, enum alarmtimer_type type)
 
 	delta = ktime_sub(absexp, base->get_ktime());
 
-	spin_lock_irqsave(&freezer_delta_lock, flags);
+	guard(spinlock_irqsave)(&freezer_delta_lock);
 	if (!freezer_delta || (delta < freezer_delta)) {
 		freezer_delta = delta;
 		freezer_expires = absexp;
 		freezer_alarmtype = type;
 	}
-	spin_unlock_irqrestore(&freezer_delta_lock, flags);
 }
 
 /**

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tip: timers/core] alarmtimer: Remove dead return value in clock2alarm()
  2025-04-30  3:27 ` [PATCH v3 2/3] alarmtimer: remove dead return value in clock2alarm() Su Hui
@ 2025-04-30  7:22   ` tip-bot2 for Su Hui
  2025-04-30 22:14   ` [PATCH v3 2/3] alarmtimer: remove " John Stultz
  1 sibling, 0 replies; 10+ messages in thread
From: tip-bot2 for Su Hui @ 2025-04-30  7:22 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Su Hui, Thomas Gleixner, x86, linux-kernel

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     d8ca84d48a2a2d4f6780980743c34b70c49f5844
Gitweb:        https://git.kernel.org/tip/d8ca84d48a2a2d4f6780980743c34b70c49f5844
Author:        Su Hui <suhui@nfschina.com>
AuthorDate:    Wed, 30 Apr 2025 11:27:33 +08:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 30 Apr 2025 09:06:23 +02:00

alarmtimer: Remove dead return value in clock2alarm()

'clockid' can only be ALARM_REALTIME and ALARM_BOOTTIME. It's impossible to
return -1 and callers never check the return value.

Only alarm_clock_get_timespec(), alarm_clock_get_ktime(),
alarm_timer_create() and alarm_timer_nsleep() call clock2alarm(). These
callers use clockid_to_kclock() to get 'struct k_clock', which ensures
that clock2alarm() never returns -1.

Remove the impossible -1 return value, and add a warning to notify about any
future misuse of this function.

Signed-off-by: Su Hui <suhui@nfschina.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/20250430032734.2079290-3-suhui@nfschina.com

---
 kernel/time/alarmtimer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 0ddccdf..621d396 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -515,9 +515,9 @@ static enum alarmtimer_type clock2alarm(clockid_t clockid)
 {
 	if (clockid == CLOCK_REALTIME_ALARM)
 		return ALARM_REALTIME;
-	if (clockid == CLOCK_BOOTTIME_ALARM)
-		return ALARM_BOOTTIME;
-	return -1;
+
+	WARN_ON_ONCE(clockid != CLOCK_BOOTTIME_ALARM);
+	return ALARM_BOOTTIME;
 }
 
 /**

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tip: timers/core] time/jiffies: Change register_refined_jiffies() to void __init
  2025-04-30  3:27 ` [PATCH v3 1/3] time/jiffies: change register_refined_jiffies() to void __init Su Hui
@ 2025-04-30  7:22   ` tip-bot2 for Su Hui
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot2 for Su Hui @ 2025-04-30  7:22 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Su Hui, Thomas Gleixner, x86, linux-kernel

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     007c07168ac0c64387be500f6604b09ace3f3bdc
Gitweb:        https://git.kernel.org/tip/007c07168ac0c64387be500f6604b09ace3f3bdc
Author:        Su Hui <suhui@nfschina.com>
AuthorDate:    Wed, 30 Apr 2025 11:27:32 +08:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 30 Apr 2025 09:06:23 +02:00

time/jiffies: Change register_refined_jiffies() to void __init

register_refined_jiffies() is only used in setup code and always returns 0.
Mark it as __init to save some bytes and change it to void.

Signed-off-by: Su Hui <suhui@nfschina.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/20250430032734.2079290-2-suhui@nfschina.com
---
 include/linux/jiffies.h | 2 +-
 kernel/time/jiffies.c   | 5 +----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
index 0ea8c98..91b2078 100644
--- a/include/linux/jiffies.h
+++ b/include/linux/jiffies.h
@@ -59,7 +59,7 @@
 /* LATCH is used in the interval timer and ftape setup. */
 #define LATCH ((CLOCK_TICK_RATE + HZ/2) / HZ)	/* For divider */
 
-extern int register_refined_jiffies(long clock_tick_rate);
+extern void register_refined_jiffies(long clock_tick_rate);
 
 /* TICK_USEC is the time between ticks in usec assuming SHIFTED_HZ */
 #define TICK_USEC ((USEC_PER_SEC + HZ/2) / HZ)
diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
index bc4db9e..34eeaca 100644
--- a/kernel/time/jiffies.c
+++ b/kernel/time/jiffies.c
@@ -75,13 +75,11 @@ struct clocksource * __init __weak clocksource_default_clock(void)
 
 static struct clocksource refined_jiffies;
 
-int register_refined_jiffies(long cycles_per_second)
+void __init register_refined_jiffies(long cycles_per_second)
 {
 	u64 nsec_per_tick, shift_hz;
 	long cycles_per_tick;
 
-
-
 	refined_jiffies = clocksource_jiffies;
 	refined_jiffies.name = "refined-jiffies";
 	refined_jiffies.rating++;
@@ -100,5 +98,4 @@ int register_refined_jiffies(long cycles_per_second)
 	refined_jiffies.mult = ((u32)nsec_per_tick) << JIFFIES_SHIFT;
 
 	__clocksource_register(&refined_jiffies);
-	return 0;
 }

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/3] alarmtimer: remove dead return value in clock2alarm()
  2025-04-30  3:27 ` [PATCH v3 2/3] alarmtimer: remove dead return value in clock2alarm() Su Hui
  2025-04-30  7:22   ` [tip: timers/core] alarmtimer: Remove " tip-bot2 for Su Hui
@ 2025-04-30 22:14   ` John Stultz
  1 sibling, 0 replies; 10+ messages in thread
From: John Stultz @ 2025-04-30 22:14 UTC (permalink / raw)
  To: Su Hui; +Cc: tglx, sboyd, linux-kernel, kernel-janitors

On Tue, Apr 29, 2025 at 8:27 PM Su Hui <suhui@nfschina.com> wrote:
>
> 'clockid' only can be ALARM_REALTIME and ALARM_BOOTTIME. It's impossible
> to return -1 and callers never check the value of -1.
>
> Only alarm_clock_get_timespec(), alarm_clock_get_ktime(),
> alarm_timer_create() and alarm_timer_nsleep() call clock2alarm(). These
> callers using clockid_to_kclock() to get 'struct k_clock', this ensures
> clock2alarm() never returns -1.
>
> Remove the impossible -1 return value, and add a warn to be aware of any
> misuse of this function.
>
> Signed-off-by: Su Hui <suhui@nfschina.com>

Acked-by: John Stultz <jstultz@google.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-04-30 22:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-30  3:27 [PATCH v3 0/3] time: some cleanup for jiffies and alarmtimer Su Hui
2025-04-30  3:27 ` [PATCH v3 1/3] time/jiffies: change register_refined_jiffies() to void __init Su Hui
2025-04-30  7:22   ` [tip: timers/core] time/jiffies: Change " tip-bot2 for Su Hui
2025-04-30  3:27 ` [PATCH v3 2/3] alarmtimer: remove dead return value in clock2alarm() Su Hui
2025-04-30  7:22   ` [tip: timers/core] alarmtimer: Remove " tip-bot2 for Su Hui
2025-04-30 22:14   ` [PATCH v3 2/3] alarmtimer: remove " John Stultz
2025-04-30  3:27 ` [PATCH v3 3/3] alarmtimer: switch some spin_{lock,unlock}_irqsave() to guard() Su Hui
2025-04-30  7:04   ` Thomas Gleixner
2025-04-30  7:19     ` Su Hui
2025-04-30  7:22   ` [tip: timers/core] alarmtimer: Switch spin_{lock,unlock}_irqsave() to guards tip-bot2 for Su Hui

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®