[PATCH 0/3] alarmtimer: Drop the spinlock around the rtcdev variable

[PATCH 0/3] alarmtimer: Drop the spinlock around the rtcdev variable

[Thomas Weißschuh]

The spinlock serves multiple purposes. It serializes calls to
alarmtimer_rtc_add_device(), making sure the 'rtcdev' variable is
written only once. It also makes the variables values is read in a
consistent state in alarmtimer_get_rtcdev().
But alarmtimer_rtc_add_device() is already synchronized externally
through the driver core's subsystem lock. So additional spinlock can be
dropped at replaced with WRITE_ONCE()/READ_ONCE() for consistent access.

This removes lock/unlock operations from the alarmtimer's clock
callbacks.
    

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Thomas Weißschuh (3):
      alarmtimer: Drop the spinlock around the rtcdev variable
      alarmtimer: Return early if platform_device_register_data() fails
      alarmtimer: Return early if try_module_get() fails

 kernel/time/alarmtimer.c | 36 ++++++++++++++----------------------
 1 file changed, 14 insertions(+), 22 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261015-alarmtimer-locking-2e7abe32a23a

Best regards,
--  
Thomas Weißschuh <linux@weissschuh.net>

[PATCH 1/3] alarmtimer: Drop the spinlock around the rtcdev variable

[Thomas Weißschuh]

The spinlock serves multiple purposes. It serializes calls to
alarmtimer_rtc_add_device(), making sure the 'rtcdev' variable is
written only once. It also makes the variables values is read in a
consistent state in alarmtimer_get_rtcdev().
But alarmtimer_rtc_add_device() is already synchronized externally
through the driver core's subsystem lock. So additional spinlock can be
dropped at replaced with WRITE_ONCE()/READ_ONCE() for consistent access.

This removes lock/unlock operations from the alarmtimer's clock
callbacks.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 kernel/time/alarmtimer.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index ea5be5870e51..e60c626e326b 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -61,7 +61,6 @@ static DEFINE_SPINLOCK(freezer_delta_lock);
 /* rtc timer and device for setting alarm wakeups at suspend */
 static struct rtc_timer		rtctimer;
 static struct rtc_device	*rtcdev;
-static DEFINE_SPINLOCK(rtcdev_lock);
 
 /**
  * alarmtimer_get_rtcdev - Return selected rtcdevice
@@ -70,12 +69,7 @@ static DEFINE_SPINLOCK(rtcdev_lock);
  */
 struct rtc_device *alarmtimer_get_rtcdev(void)
 {
-	struct rtc_device *ret;
-
-	guard(spinlock_irqsave)(&rtcdev_lock);
-	ret = rtcdev;
-
-	return ret;
+	return READ_ONCE(rtcdev);
 }
 EXPORT_SYMBOL_GPL(alarmtimer_get_rtcdev);
 
@@ -98,15 +92,13 @@ static int alarmtimer_rtc_add_device(struct device *dev)
 	if (!IS_ERR(pdev))
 		device_init_wakeup(&pdev->dev, true);
 
-	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;
-		}
+	if (!IS_ERR(pdev) && try_module_get(rtc->owner)) {
+		WRITE_ONCE(rtcdev, rtc);
+		/* hold a reference so it doesn't go away */
+		get_device(dev);
+		pdev = NULL;
+	} else {
+		ret = -1;
 	}
 
 	platform_device_unregister(pdev);

-- 
2.56.0

[PATCH 2/3] alarmtimer: Return early if platform_device_register_data() fails

[Thomas Weißschuh]

Simplify the logic by returning on error instead of having to handle the
potential error in each following operation.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 kernel/time/alarmtimer.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index e60c626e326b..9ab1b053a2fb 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -89,10 +89,12 @@ static int alarmtimer_rtc_add_device(struct device *dev)
 
 	pdev = platform_device_register_data(dev, "alarmtimer",
 					     PLATFORM_DEVID_AUTO, NULL, 0);
-	if (!IS_ERR(pdev))
-		device_init_wakeup(&pdev->dev, true);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
 
-	if (!IS_ERR(pdev) && try_module_get(rtc->owner)) {
+	device_init_wakeup(&pdev->dev, true);
+
+	if (try_module_get(rtc->owner)) {
 		WRITE_ONCE(rtcdev, rtc);
 		/* hold a reference so it doesn't go away */
 		get_device(dev);

-- 
2.56.0

[PATCH 3/3] alarmtimer: Return early if try_module_get() fails

[Thomas Weißschuh]

Simplify the logic by returning early if try_module_get() fails.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 kernel/time/alarmtimer.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 9ab1b053a2fb..b1ae0d2c6a7d 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -77,7 +77,6 @@ static int alarmtimer_rtc_add_device(struct device *dev)
 {
 	struct rtc_device *rtc = to_rtc_device(dev);
 	struct platform_device *pdev;
-	int ret = 0;
 
 	if (rtcdev)
 		return -EBUSY;
@@ -94,17 +93,16 @@ static int alarmtimer_rtc_add_device(struct device *dev)
 
 	device_init_wakeup(&pdev->dev, true);
 
-	if (try_module_get(rtc->owner)) {
-		WRITE_ONCE(rtcdev, rtc);
-		/* hold a reference so it doesn't go away */
-		get_device(dev);
-		pdev = NULL;
-	} else {
-		ret = -1;
+	if (!try_module_get(rtc->owner)) {
+		platform_device_unregister(pdev);
+		return -1;
 	}
 
-	platform_device_unregister(pdev);
-	return ret;
+	WRITE_ONCE(rtcdev, rtc);
+	/* hold a reference so it doesn't go away */
+	get_device(dev);
+
+	return 0;
 }
 
 static inline void alarmtimer_rtc_timer_init(void)

-- 
2.56.0

Re: [PATCH 3/3] alarmtimer: Return early if try_module_get() fails

[Thomas Gleixner]

On Mon, Nov 02 2026 at 06:11, Thomas Weißschuh wrote:
> @@ -94,17 +93,16 @@ static int alarmtimer_rtc_add_device(struct device *dev)
>  
>  	device_init_wakeup(&pdev->dev, true);

The wakeup functionality is enabled _before_ the last condition
try_module_get() is evaluated. That doesn't make any sense.

[tip: timers/core] alarmtimer: Return early if try_module_get() fails

[tip-bot2 for Thomas Weißschuh]

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

Commit-ID:     8c35d628b82dfa883394fa7cbfb9847017ed3d52
Gitweb:        https://git.kernel.org/tip/8c35d628b82dfa883394fa7cbfb9847017ed3d52
Author:        Thomas Weißschuh <linux@weissschuh.net>
AuthorDate:    Mon, 02 Nov 2026 06:11:56 
Committer:     Thomas Gleixner <tglx@kernel.org>
CommitterDate: Mon, 09 Nov 2026 20:50:03 +01:00

alarmtimer: Return early if try_module_get() fails

Simplify the logic by returning early if try_module_get() fails.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Link: https://patch.msgid.link/20261102-alarmtimer-locking-v1-3-3c79a698aeae@weissschuh.net
---
 kernel/time/alarmtimer.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 8eeea09..33c35f1 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -77,7 +77,6 @@ static int alarmtimer_rtc_add_device(struct device *dev)
 {
 	struct rtc_device *rtc = to_rtc_device(dev);
 	struct platform_device *pdev;
-	int ret = 0;
 
 	if (rtcdev)
 		return -EBUSY;
@@ -94,17 +93,16 @@ static int alarmtimer_rtc_add_device(struct device *dev)
 
 	device_init_wakeup(&pdev->dev, true);
 
-	if (try_module_get(rtc->owner)) {
-		WRITE_ONCE(rtcdev, rtc);
-		/* hold a reference so it doesn't go away */
-		get_device(dev);
-		pdev = NULL;
-	} else {
-		ret = -1;
+	if (!try_module_get(rtc->owner)) {
+		platform_device_unregister(pdev);
+		return -1;
 	}
 
-	platform_device_unregister(pdev);
-	return ret;
+	WRITE_ONCE(rtcdev, rtc);
+	/* Hold a reference so it doesn't go away */
+	get_device(dev);
+
+	return 0;
 }
 
 static inline void alarmtimer_rtc_timer_init(void)

[tip: timers/core] alarmtimer: Return early if platform_device_register_data() fails

[tip-bot2 for Thomas Weißschuh]

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

Commit-ID:     4ed8ae10367e88a974582027494ce4a197c5b175
Gitweb:        https://git.kernel.org/tip/4ed8ae10367e88a974582027494ce4a197c5b175
Author:        Thomas Weißschuh <linux@weissschuh.net>
AuthorDate:    Mon, 02 Nov 2026 06:11:55 
Committer:     Thomas Gleixner <tglx@kernel.org>
CommitterDate: Mon, 09 Nov 2026 20:50:03 +01:00

alarmtimer: Return early if platform_device_register_data() fails

Simplify the logic by returning on error instead of having to handle the
potential error in each following operation.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Link: https://patch.msgid.link/20261102-alarmtimer-locking-v1-2-3c79a698aeae@weissschuh.net
---
 kernel/time/alarmtimer.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 601ebdc..8eeea09 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -89,10 +89,12 @@ static int alarmtimer_rtc_add_device(struct device *dev)
 
 	pdev = platform_device_register_data(dev, "alarmtimer",
 					     PLATFORM_DEVID_AUTO, NULL, 0);
-	if (!IS_ERR(pdev))
-		device_init_wakeup(&pdev->dev, true);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
 
-	if (!IS_ERR(pdev) && try_module_get(rtc->owner)) {
+	device_init_wakeup(&pdev->dev, true);
+
+	if (try_module_get(rtc->owner)) {
 		WRITE_ONCE(rtcdev, rtc);
 		/* hold a reference so it doesn't go away */
 		get_device(dev);

[tip: timers/core] alarmtimer: Drop the spinlock around the rtcdev variable

[tip-bot2 for Thomas Weißschuh]

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

Commit-ID:     6d771f75f3761d7c8aaf57b06a2b4ddcb25b4ce8
Gitweb:        https://git.kernel.org/tip/6d771f75f3761d7c8aaf57b06a2b4ddcb25b4ce8
Author:        Thomas Weißschuh <linux@weissschuh.net>
AuthorDate:    Mon, 02 Nov 2026 06:11:54 
Committer:     Thomas Gleixner <tglx@kernel.org>
CommitterDate: Mon, 09 Nov 2026 20:50:03 +01:00

alarmtimer: Drop the spinlock around the rtcdev variable

The spinlock serializes calls to alarmtimer_rtc_add_device(), making sure
the 'rtcdev' variable is written only once. It also protects the read of
the 'rtcdev' variable in alarmtimer_get_rtcdev().

But alarmtimer_rtc_add_device() is already synchronized externally through
the driver core's subsystem lock. So the additional spinlock can be dropped
and replaced with WRITE_ONCE()/READ_ONCE() for consistent access.

This removes lock/unlock operations from the alarmtimer's clock callbacks.

[ tglx: Massage change log ]

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Link: https://patch.msgid.link/20261102-alarmtimer-locking-v1-1-3c79a698aeae@weissschuh.net
---
 kernel/time/alarmtimer.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index d6dc6e8..601ebdc 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -61,7 +61,6 @@ static DEFINE_SPINLOCK(freezer_delta_lock);
 /* rtc timer and device for setting alarm wakeups at suspend */
 static struct rtc_timer		rtctimer;
 static struct rtc_device	*rtcdev;
-static DEFINE_SPINLOCK(rtcdev_lock);
 
 /**
  * alarmtimer_get_rtcdev - Return selected rtcdevice
@@ -70,12 +69,7 @@ static DEFINE_SPINLOCK(rtcdev_lock);
  */
 struct rtc_device *alarmtimer_get_rtcdev(void)
 {
-	struct rtc_device *ret;
-
-	guard(spinlock_irqsave)(&rtcdev_lock);
-	ret = rtcdev;
-
-	return ret;
+	return READ_ONCE(rtcdev);
 }
 EXPORT_SYMBOL_GPL(alarmtimer_get_rtcdev);
 
@@ -98,15 +92,13 @@ static int alarmtimer_rtc_add_device(struct device *dev)
 	if (!IS_ERR(pdev))
 		device_init_wakeup(&pdev->dev, true);
 
-	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;
-		}
+	if (!IS_ERR(pdev) && try_module_get(rtc->owner)) {
+		WRITE_ONCE(rtcdev, rtc);
+		/* hold a reference so it doesn't go away */
+		get_device(dev);
+		pdev = NULL;
+	} else {
+		ret = -1;
 	}
 
 	platform_device_unregister(pdev);