Hi Sebastian,
FYI, the error/warning still remains.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: b8f1fa2419c19c81bc386a6b350879ba54a573e1
commit: 73d20564e0dcae003e0d79977f044d5e57496304 hrtimer: Don't dereference the hrtimer pointer after the callback
date: 3 years, 8 months ago
config: i386-allnoconfig (https://download.01.org/0day-ci/archive/20231119/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231119/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
kernel/time/hrtimer.c:120:35: warning: initialized field overwritten [-Woverride-init]
120 | [CLOCK_REALTIME] = HRTIMER_BASE_REALTIME,
| ^~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:120:35: note: (near initialization for 'hrtimer_clock_to_base_table[0]')
kernel/time/hrtimer.c:121:35: warning: initialized field overwritten [-Woverride-init]
121 | [CLOCK_MONOTONIC] = HRTIMER_BASE_MONOTONIC,
| ^~~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:121:35: note: (near initialization for 'hrtimer_clock_to_base_table[1]')
kernel/time/hrtimer.c:122:35: warning: initialized field overwritten [-Woverride-init]
122 | [CLOCK_BOOTTIME] = HRTIMER_BASE_BOOTTIME,
| ^~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:122:35: note: (near initialization for 'hrtimer_clock_to_base_table[7]')
kernel/time/hrtimer.c:123:35: warning: initialized field overwritten [-Woverride-init]
123 | [CLOCK_TAI] = HRTIMER_BASE_TAI,
| ^~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:123:35: note: (near initialization for 'hrtimer_clock_to_base_table[11]')
kernel/time/hrtimer.c: In function '__run_hrtimer':
>> kernel/time/hrtimer.c:1483:14: warning: variable 'expires_in_hardirq' set but not used [-Wunused-but-set-variable]
1483 | bool expires_in_hardirq;
| ^~~~~~~~~~~~~~~~~~
vim +/expires_in_hardirq +1483 kernel/time/hrtimer.c
1458
1459 /*
1460 * The write_seqcount_barrier()s in __run_hrtimer() split the thing into 3
1461 * distinct sections:
1462 *
1463 * - queued: the timer is queued
1464 * - callback: the timer is being ran
1465 * - post: the timer is inactive or (re)queued
1466 *
1467 * On the read side we ensure we observe timer->state and cpu_base->running
1468 * from the same section, if anything changed while we looked at it, we retry.
1469 * This includes timer->base changing because sequence numbers alone are
1470 * insufficient for that.
1471 *
1472 * The sequence numbers are required because otherwise we could still observe
1473 * a false negative if the read side got smeared over multiple consequtive
1474 * __run_hrtimer() invocations.
1475 */
1476
1477 static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base,
1478 struct hrtimer_clock_base *base,
1479 struct hrtimer *timer, ktime_t *now,
1480 unsigned long flags) __must_hold(&cpu_base->lock)
1481 {
1482 enum hrtimer_restart (*fn)(struct hrtimer *);
> 1483 bool expires_in_hardirq;
1484 int restart;
1485
1486 lockdep_assert_held(&cpu_base->lock);
1487
1488 debug_deactivate(timer);
1489 base->running = timer;
1490
1491 /*
1492 * Separate the ->running assignment from the ->state assignment.
1493 *
1494 * As with a regular write barrier, this ensures the read side in
1495 * hrtimer_active() cannot observe base->running == NULL &&
1496 * timer->state == INACTIVE.
1497 */
1498 raw_write_seqcount_barrier(&base->seq);
1499
1500 __remove_hrtimer(timer, base, HRTIMER_STATE_INACTIVE, 0);
1501 fn = timer->function;
1502
1503 /*
1504 * Clear the 'is relative' flag for the TIME_LOW_RES case. If the
1505 * timer is restarted with a period then it becomes an absolute
1506 * timer. If its not restarted it does not matter.
1507 */
1508 if (IS_ENABLED(CONFIG_TIME_LOW_RES))
1509 timer->is_rel = false;
1510
1511 /*
1512 * The timer is marked as running in the CPU base, so it is
1513 * protected against migration to a different CPU even if the lock
1514 * is dropped.
1515 */
1516 raw_spin_unlock_irqrestore(&cpu_base->lock, flags);
1517 trace_hrtimer_expire_entry(timer, now);
1518 expires_in_hardirq = lockdep_hrtimer_enter(timer);
1519
1520 restart = fn(timer);
1521
1522 lockdep_hrtimer_exit(expires_in_hardirq);
1523 trace_hrtimer_expire_exit(timer);
1524 raw_spin_lock_irq(&cpu_base->lock);
1525
1526 /*
1527 * Note: We clear the running state after enqueue_hrtimer and
1528 * we do not reprogram the event hardware. Happens either in
1529 * hrtimer_start_range_ns() or in hrtimer_interrupt()
1530 *
1531 * Note: Because we dropped the cpu_base->lock above,
1532 * hrtimer_start_range_ns() can have popped in and enqueued the timer
1533 * for us already.
1534 */
1535 if (restart != HRTIMER_NORESTART &&
1536 !(timer->state & HRTIMER_STATE_ENQUEUED))
1537 enqueue_hrtimer(timer, base, HRTIMER_MODE_ABS);
1538
1539 /*
1540 * Separate the ->running assignment from the ->state assignment.
1541 *
1542 * As with a regular write barrier, this ensures the read side in
1543 * hrtimer_active() cannot observe base->running.timer == NULL &&
1544 * timer->state == INACTIVE.
1545 */
1546 raw_write_seqcount_barrier(&base->seq);
1547
1548 WARN_ON_ONCE(base->running != timer);
1549 base->running = NULL;
1550 }
1551
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 2023-11-12 08:52:44 [+0800], kernel test robot wrote:
> Hi Sebastian,
Hi,
> FYI, the error/warning still remains.
>
> kernel/time/hrtimer.c:120:35: warning: initialized field overwritten [-Woverride-init]
> 120 | [CLOCK_REALTIME] = HRTIMER_BASE_REALTIME,
> | ^~~~~~~~~~~~~~~~~~~~~
> kernel/time/hrtimer.c:120:35: note: (near initialization for 'hrtimer_clock_to_base_table[0]')
> kernel/time/hrtimer.c:121:35: warning: initialized field overwritten [-Woverride-init]
> 121 | [CLOCK_MONOTONIC] = HRTIMER_BASE_MONOTONIC,
> | ^~~~~~~~~~~~~~~~~~~~~~
> kernel/time/hrtimer.c:121:35: note: (near initialization for 'hrtimer_clock_to_base_table[1]')
> kernel/time/hrtimer.c:122:35: warning: initialized field overwritten [-Woverride-init]
> 122 | [CLOCK_BOOTTIME] = HRTIMER_BASE_BOOTTIME,
> | ^~~~~~~~~~~~~~~~~~~~~
> kernel/time/hrtimer.c:122:35: note: (near initialization for 'hrtimer_clock_to_base_table[7]')
> kernel/time/hrtimer.c:123:35: warning: initialized field overwritten [-Woverride-init]
> 123 | [CLOCK_TAI] = HRTIMER_BASE_TAI,
> | ^~~~~~~~~~~~~~~~
> kernel/time/hrtimer.c:123:35: note: (near initialization for 'hrtimer_clock_to_base_table[11]')
This is "okay". The array is first initialized completely to a safe
value and then a few of the individual values are overwritten.
So this "initialized field overwritten", that the compiler complains
about, is intended here.
> kernel/time/hrtimer.c: In function '__run_hrtimer':
> >> kernel/time/hrtimer.c:1483:14: warning: variable 'expires_in_hardirq' set but not used [-Wunused-but-set-variable]
> 1483 | bool expires_in_hardirq;
> | ^~~~~~~~~~~~~~~~~~
This is because the code using it is disabled at build time. I could
tweak the code to make look it used in the off case so the warning goes
away.
Sebastian
The lockdep_hrtimer_enter() macro returns a value which is then saved in
a local variable. The value is then fed to lockdep_hrtimer_exit(). In
case lockdep is disabled then the value remains unused leading to a "set
but not used" warning by the compiler.
Make the content passed to lockdep_hrtimer_exit() look used so the
compiler does not complain about it.
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
include/linux/irqflags.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index 2b665c32f5fe6..2e09c269bf9d8 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -126,7 +126,7 @@ do { \
# define lockdep_softirq_enter() do { } while (0)
# define lockdep_softirq_exit() do { } while (0)
# define lockdep_hrtimer_enter(__hrtimer) false
-# define lockdep_hrtimer_exit(__context) do { } while (0)
+# define lockdep_hrtimer_exit(__context) do { (void)(__context); } while (0)
# define lockdep_posixtimer_enter() do { } while (0)
# define lockdep_posixtimer_exit() do { } while (0)
# define lockdep_irq_work_enter(__work) do { } while (0)
--
2.42.0