rtc: cmos: avoid taking rtc_lock for extended period of time
PATCH 1/1 rtc: cmos: avoid taking rtc_lock for extended period of time
On my device reading entirety of /sys/devices/pnp0/00:03/cmos_nvram0/nvmem takes about 9 msec during which time interrupts are off on the CPU that does the read and the thread that performs the read can not be migrated or preempted by another higher priority thread (RT or not).
Allow readers and writers be preempted by taking and releasing rtc_lock spinlock for each individual byte read or written rather than once per read/write request.
---
v2: use "return 0;" instead of "return count;" (which should be the same
behavior-wise because count is 0 by that point) to show intent clearer (per feedback from Mateusz Jończyk)
v1: https://lore.kernel.org/r/[email protected]
drivers/rtc/rtc-cmos.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)
drivers/rtc/rtc-cmos.c +15 -16
@@ -645,18 +645,17 @@ static int cmos_nvram_read(void *priv, unsigned int off, void *val,unsigned char *buf = val; off +=NVRAM_OFFSET ; -spin_lock_irq (&rtc_lock); -for (; count; count--, off++) { +for (; count; count--, off++, buf++) { +guard (spinlock_irq)(&rtc_lock);if (off <128 ) - *buf++ =CMOS_READ (off); + *buf =CMOS_READ (off);else if (can_bank2) - *buf++ =cmos_read_bank2 (off); + *buf =cmos_read_bank2 (off);else -break ; +return -EIO ; } -spin_unlock_irq (&rtc_lock); -return count ? -EIO :0 ; +return 0 ; } static int cmos_nvram_write(void *priv, unsigned int off, void *val,
@@ -671,23 +670,23 @@ static int cmos_nvram_write(void *priv, unsigned int off, void *val,* NVRAM to update, updating checksums is also part of its job. */ off +=NVRAM_OFFSET ; -spin_lock_irq (&rtc_lock); -for (; count; count--, off++) { +for (; count; count--, off++, buf++) {/* don't trash RTC registers */ if (off == cmos->day_alrm || off == cmos->mon_alrm || off == cmos->century) - buf++; -else if (off <128 ) -CMOS_WRITE (*buf++, off); +continue ; + +guard (spinlock_irq)(&rtc_lock); +if (off <128 ) +CMOS_WRITE (*buf, off);else if (can_bank2) -cmos_write_bank2 (*buf++, off); +cmos_write_bank2 (*buf, off);else -break ; +return -EIO ; } -spin_unlock_irq (&rtc_lock); -return count ? -EIO :0 ; +return 0 ; }/*----------------------------------------------------------------*/
Signature · 5 lines
2.47.0.163.g1226f6d8fa-goog -- Dmitry
W dniu 25.10.2024 o 22:14, Dmitry Torokhov pisze:
Quoted 10 lines
On my device reading entirety of /sys/devices/pnp0/00:03/cmos_nvram0/nvmem takes about 9 msec during which time interrupts are off on the CPU that does the read and the thread that performs the read can not be migrated or preempted by another higher priority thread (RT or not).
Allow readers and writers be preempted by taking and releasing
rtc_lockspinlock for each individual byte read or written rather than once per read/write request.Signed-off-by Dmitry Torokhov <[email protected]>
FWIW, there is a similar latency problem in hpet_rtc_interrupt() in arch/x86/kernel/hpet.c as mc146818_get_time() can sleep for up to 10ms. It should be possible to avoid using full mc146818_get_time() there and implement only what's needed (including aborting early if RTC_UIP is set etc.).
Greetings,
Mateusz
Quoted 10 lines · Dmitry Torokhov
On my device reading entirety of /sys/devices/pnp0/00:03/cmos_nvram0/nvmem takes about 9 msec during which time interrupts are off on the CPU that does the read and the thread that performs the read can not be migrated or preempted by another higher priority thread (RT or not).
Allow readers and writers be preempted by taking and releasing
rtc_lockspinlock for each individual byte read or written rather than once per read/write request.[...]
Applied, thanks!
[1/1] rtc: cmos: avoid taking rtc_lock for extended period of time
https://git.kernel.org/abelloni/c/0a6efab33eab
Best regards,
Signature · 3 lines
Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com