1

Misc timer fixes:

- Address a (valid) W=1 build warning
 
  - Fix timer self-tests
 
  - Annotate a KCSAN warning wrt. accesses to the
    tick_do_timer_cpu global variable.
 
  - Address a !CONFIG_BUG build warning
 
 Heads up for the !CONFIG_BUG warning patch, which we
 addressed with:
 
    5284984a4f bug: Fix no-return-statement warning with !CONFIG_BUG
 
 Not everyone agreed though, see:
 
   https://lore.kernel.org/all/20240410153212.127477-1-adrian.hunter@intel.com
 
 Signed-off-by: Ingo Molnar <mingo@kernel.org>
 -----BEGIN PGP SIGNATURE-----
 
 iQJFBAABCgAvFiEEBpT5eoXrXCwVQwEKEnMQ0APhK1gFAmYblgkRHG1pbmdvQGtl
 cm5lbC5vcmcACgkQEnMQ0APhK1jYWg//eNeJkdzJVdbj6g4n2t3WDDuX7dxuRqdG
 AQHJdZctG+kNZBp+U2Zvbb8BDZfDRSQDBDfQI0ck3xG314pzXzNg92YMJB95r/Zf
 aRcxMSFc3a2dN3vW97UDKquPuCarCPsZQvbQKmZ55OmgW6ZRhhsjed0f18Nq63xR
 oWrQ0rotNhMJ98dpSOfPqrMoCXza78P/7nA49LxVIQcuDb+dtyqVTuAbENOOkFYq
 nqAkvuieZGzLb4nKH2d1rK4agYuXwnMLJ71MOcCNWFp8njuRRx+Yc+3gyoNl7e9E
 ipd6DcelOEl/DaYRao9rRy3ij0veJoUvshKZBTEWPw9FQU24odwqX4p/Mj2vF1iN
 KExtF+S7LBxdJAdivHyuPtt9B0rKRmgIp/Q8Ytgzuxu9rZ3LNev+7l80qDOIM8MF
 Mozv6JsJN2sVOMWvnzF9B1WNjVSikcyuvd2JRPbQYh1zy8aCpFHhZY+LcvK3vYBQ
 qdzY8o5dmIW0JrtHZw4H7tqKByUKEbJMsslPefD9qNIq5bpAUgHi7HFOMTU0kOvx
 2rFDnC6cJk39CXyJrpLMyKDqZzDTHGV/J4nV7/L7vQzy3iIOcfVcszfGESaM/txk
 6cgdncf9pr8aOE34A6/5Kr4L45vgh7B6YGc4oqHpdlvFLR0ve0gi+BIjNja8Jy7C
 IwGsS2uloCA=
 =oEym
 -----END PGP SIGNATURE-----

Merge tag 'timers-urgent-2024-04-14' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip

Pull timer fixes from Ingo Molnar:

 - Address a (valid) W=1 build warning

 - Fix timer self-tests

 - Annotate a KCSAN warning wrt. accesses to the tick_do_timer_cpu
   global variable

 - Address a !CONFIG_BUG build warning

* tag 'timers-urgent-2024-04-14' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip:
  selftests: kselftest: Fix build failure with NOLIBC
  selftests: timers: Fix abs() warning in posix_timers test
  selftests: kselftest: Mark functions that unconditionally call exit() as __noreturn
  selftests: timers: Fix posix_timers ksft_print_msg() warning
  selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior
  bug: Fix no-return-statement warning with !CONFIG_BUG
  timekeeping: Use READ/WRITE_ONCE() for tick_do_timer_cpu
  selftests/timers/posix_timers: Reimplement check_timer_distribution()
  irqflags: Explicitly ignore lockdep_hrtimer_exit() argument
This commit is contained in:
Linus Torvalds 2024-04-14 10:32:22 -07:00
commit c748fc3b1f
7 changed files with 147 additions and 124 deletions

View File

@ -156,7 +156,10 @@ extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
#else /* !CONFIG_BUG */
#ifndef HAVE_ARCH_BUG
#define BUG() do {} while (1)
#define BUG() do { \
do {} while (1); \
unreachable(); \
} while (0)
#endif
#ifndef HAVE_ARCH_BUG_ON

View File

@ -114,7 +114,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)

View File

@ -7,6 +7,7 @@
* Copyright(C) 2005-2007, Red Hat, Inc., Ingo Molnar
* Copyright(C) 2006-2007, Timesys Corp., Thomas Gleixner
*/
#include <linux/compiler.h>
#include <linux/cpu.h>
#include <linux/err.h>
#include <linux/hrtimer.h>
@ -84,7 +85,7 @@ int tick_is_oneshot_available(void)
*/
static void tick_periodic(int cpu)
{
if (tick_do_timer_cpu == cpu) {
if (READ_ONCE(tick_do_timer_cpu) == cpu) {
raw_spin_lock(&jiffies_lock);
write_seqcount_begin(&jiffies_seq);
@ -215,8 +216,8 @@ static void tick_setup_device(struct tick_device *td,
* If no cpu took the do_timer update, assign it to
* this cpu:
*/
if (tick_do_timer_cpu == TICK_DO_TIMER_BOOT) {
tick_do_timer_cpu = cpu;
if (READ_ONCE(tick_do_timer_cpu) == TICK_DO_TIMER_BOOT) {
WRITE_ONCE(tick_do_timer_cpu, cpu);
tick_next_period = ktime_get();
#ifdef CONFIG_NO_HZ_FULL
/*
@ -232,7 +233,7 @@ static void tick_setup_device(struct tick_device *td,
!tick_nohz_full_cpu(cpu)) {
tick_take_do_timer_from_boot();
tick_do_timer_boot_cpu = -1;
WARN_ON(tick_do_timer_cpu != cpu);
WARN_ON(READ_ONCE(tick_do_timer_cpu) != cpu);
#endif
}
@ -406,10 +407,10 @@ void tick_assert_timekeeping_handover(void)
int tick_cpu_dying(unsigned int dying_cpu)
{
/*
* If the current CPU is the timekeeper, it's the only one that
* can safely hand over its duty. Also all online CPUs are in
* stop machine, guaranteed not to be idle, therefore it's safe
* to pick any online successor.
* If the current CPU is the timekeeper, it's the only one that can
* safely hand over its duty. Also all online CPUs are in stop
* machine, guaranteed not to be idle, therefore there is no
* concurrency and it's safe to pick any online successor.
*/
if (tick_do_timer_cpu == dying_cpu)
tick_do_timer_cpu = cpumask_first(cpu_online_mask);

View File

@ -8,6 +8,7 @@
*
* Started by: Thomas Gleixner and Ingo Molnar
*/
#include <linux/compiler.h>
#include <linux/cpu.h>
#include <linux/err.h>
#include <linux/hrtimer.h>
@ -204,7 +205,7 @@ static inline void tick_sched_flag_clear(struct tick_sched *ts,
static void tick_sched_do_timer(struct tick_sched *ts, ktime_t now)
{
int cpu = smp_processor_id();
int tick_cpu, cpu = smp_processor_id();
/*
* Check if the do_timer duty was dropped. We don't care about
@ -216,16 +217,18 @@ static void tick_sched_do_timer(struct tick_sched *ts, ktime_t now)
* If nohz_full is enabled, this should not happen because the
* 'tick_do_timer_cpu' CPU never relinquishes.
*/
if (IS_ENABLED(CONFIG_NO_HZ_COMMON) &&
unlikely(tick_do_timer_cpu == TICK_DO_TIMER_NONE)) {
tick_cpu = READ_ONCE(tick_do_timer_cpu);
if (IS_ENABLED(CONFIG_NO_HZ_COMMON) && unlikely(tick_cpu == TICK_DO_TIMER_NONE)) {
#ifdef CONFIG_NO_HZ_FULL
WARN_ON_ONCE(tick_nohz_full_running);
#endif
tick_do_timer_cpu = cpu;
WRITE_ONCE(tick_do_timer_cpu, cpu);
tick_cpu = cpu;
}
/* Check if jiffies need an update */
if (tick_do_timer_cpu == cpu)
if (tick_cpu == cpu)
tick_do_update_jiffies64(now);
/*
@ -610,7 +613,7 @@ bool tick_nohz_cpu_hotpluggable(unsigned int cpu)
* timers, workqueues, timekeeping, ...) on behalf of full dynticks
* CPUs. It must remain online when nohz full is enabled.
*/
if (tick_nohz_full_running && tick_do_timer_cpu == cpu)
if (tick_nohz_full_running && READ_ONCE(tick_do_timer_cpu) == cpu)
return false;
return true;
}
@ -891,6 +894,7 @@ static ktime_t tick_nohz_next_event(struct tick_sched *ts, int cpu)
{
u64 basemono, next_tick, delta, expires;
unsigned long basejiff;
int tick_cpu;
basemono = get_jiffies_update(&basejiff);
ts->last_jiffies = basejiff;
@ -947,9 +951,9 @@ static ktime_t tick_nohz_next_event(struct tick_sched *ts, int cpu)
* Otherwise we can sleep as long as we want.
*/
delta = timekeeping_max_deferment();
if (cpu != tick_do_timer_cpu &&
(tick_do_timer_cpu != TICK_DO_TIMER_NONE ||
!tick_sched_flag_test(ts, TS_FLAG_DO_TIMER_LAST)))
tick_cpu = READ_ONCE(tick_do_timer_cpu);
if (tick_cpu != cpu &&
(tick_cpu != TICK_DO_TIMER_NONE || !tick_sched_flag_test(ts, TS_FLAG_DO_TIMER_LAST)))
delta = KTIME_MAX;
/* Calculate the next expiry time */
@ -970,6 +974,7 @@ static void tick_nohz_stop_tick(struct tick_sched *ts, int cpu)
unsigned long basejiff = ts->last_jiffies;
u64 basemono = ts->timer_expires_base;
bool timer_idle = tick_sched_flag_test(ts, TS_FLAG_STOPPED);
int tick_cpu;
u64 expires;
/* Make sure we won't be trying to stop it twice in a row. */
@ -1007,10 +1012,11 @@ static void tick_nohz_stop_tick(struct tick_sched *ts, int cpu)
* do_timer() never gets invoked. Keep track of the fact that it
* was the one which had the do_timer() duty last.
*/
if (cpu == tick_do_timer_cpu) {
tick_do_timer_cpu = TICK_DO_TIMER_NONE;
tick_cpu = READ_ONCE(tick_do_timer_cpu);
if (tick_cpu == cpu) {
WRITE_ONCE(tick_do_timer_cpu, TICK_DO_TIMER_NONE);
tick_sched_flag_set(ts, TS_FLAG_DO_TIMER_LAST);
} else if (tick_do_timer_cpu != TICK_DO_TIMER_NONE) {
} else if (tick_cpu != TICK_DO_TIMER_NONE) {
tick_sched_flag_clear(ts, TS_FLAG_DO_TIMER_LAST);
}
@ -1173,15 +1179,17 @@ static bool can_stop_idle_tick(int cpu, struct tick_sched *ts)
return false;
if (tick_nohz_full_enabled()) {
int tick_cpu = READ_ONCE(tick_do_timer_cpu);
/*
* Keep the tick alive to guarantee timekeeping progression
* if there are full dynticks CPUs around
*/
if (tick_do_timer_cpu == cpu)
if (tick_cpu == cpu)
return false;
/* Should not happen for nohz-full */
if (WARN_ON_ONCE(tick_do_timer_cpu == TICK_DO_TIMER_NONE))
if (WARN_ON_ONCE(tick_cpu == TICK_DO_TIMER_NONE))
return false;
}

View File

@ -51,6 +51,7 @@
#include <stdarg.h>
#include <string.h>
#include <stdio.h>
#include <sys/utsname.h>
#endif
#ifndef ARRAY_SIZE
@ -79,6 +80,9 @@
#define KSFT_XPASS 3
#define KSFT_SKIP 4
#ifndef __noreturn
#define __noreturn __attribute__((__noreturn__))
#endif
#define __printf(a, b) __attribute__((format(printf, a, b)))
/* counters */
@ -299,13 +303,13 @@ void ksft_test_result_code(int exit_code, const char *test_name,
va_end(args);
}
static inline int ksft_exit_pass(void)
static inline __noreturn int ksft_exit_pass(void)
{
ksft_print_cnts();
exit(KSFT_PASS);
}
static inline int ksft_exit_fail(void)
static inline __noreturn int ksft_exit_fail(void)
{
ksft_print_cnts();
exit(KSFT_FAIL);
@ -332,7 +336,7 @@ static inline int ksft_exit_fail(void)
ksft_cnt.ksft_xfail + \
ksft_cnt.ksft_xskip)
static inline __printf(1, 2) int ksft_exit_fail_msg(const char *msg, ...)
static inline __noreturn __printf(1, 2) int ksft_exit_fail_msg(const char *msg, ...)
{
int saved_errno = errno;
va_list args;
@ -347,19 +351,19 @@ static inline __printf(1, 2) int ksft_exit_fail_msg(const char *msg, ...)
exit(KSFT_FAIL);
}
static inline int ksft_exit_xfail(void)
static inline __noreturn int ksft_exit_xfail(void)
{
ksft_print_cnts();
exit(KSFT_XFAIL);
}
static inline int ksft_exit_xpass(void)
static inline __noreturn int ksft_exit_xpass(void)
{
ksft_print_cnts();
exit(KSFT_XPASS);
}
static inline __printf(1, 2) int ksft_exit_skip(const char *msg, ...)
static inline __noreturn __printf(1, 2) int ksft_exit_skip(const char *msg, ...)
{
int saved_errno = errno;
va_list args;
@ -388,4 +392,21 @@ static inline __printf(1, 2) int ksft_exit_skip(const char *msg, ...)
exit(KSFT_SKIP);
}
static inline int ksft_min_kernel_version(unsigned int min_major,
unsigned int min_minor)
{
#ifdef NOLIBC
ksft_print_msg("NOLIBC: Can't check kernel version: Function not implemented\n");
return 0;
#else
unsigned int major, minor;
struct utsname info;
if (uname(&info) || sscanf(info.release, "%u.%u.", &major, &minor) != 2)
ksft_exit_fail_msg("Can't parse kernel version\n");
return major > min_major || (major == min_major && minor >= min_minor);
#endif
}
#endif /* __KSELFTEST_H */

View File

@ -66,7 +66,7 @@ static int check_diff(struct timeval start, struct timeval end)
diff = end.tv_usec - start.tv_usec;
diff += (end.tv_sec - start.tv_sec) * USECS_PER_SEC;
if (abs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2) {
if (llabs(diff - DELAY * USECS_PER_SEC) > USECS_PER_SEC / 2) {
printf("Diff too high: %lld..", diff);
return -1;
}
@ -184,80 +184,71 @@ static int check_timer_create(int which)
return 0;
}
int remain;
__thread int got_signal;
static pthread_t ctd_thread;
static volatile int ctd_count, ctd_failed;
static void *distribution_thread(void *arg)
static void ctd_sighandler(int sig)
{
while (__atomic_load_n(&remain, __ATOMIC_RELAXED));
return NULL;
if (pthread_self() != ctd_thread)
ctd_failed = 1;
ctd_count--;
}
static void distribution_handler(int nr)
static void *ctd_thread_func(void *arg)
{
if (!__atomic_exchange_n(&got_signal, 1, __ATOMIC_RELAXED))
__atomic_fetch_sub(&remain, 1, __ATOMIC_RELAXED);
}
/*
* Test that all running threads _eventually_ receive CLOCK_PROCESS_CPUTIME_ID
* timer signals. This primarily tests that the kernel does not favour any one.
*/
static int check_timer_distribution(void)
{
int err, i;
timer_t id;
const int nthreads = 10;
pthread_t threads[nthreads];
struct itimerspec val = {
.it_value.tv_sec = 0,
.it_value.tv_nsec = 1000 * 1000,
.it_interval.tv_sec = 0,
.it_interval.tv_nsec = 1000 * 1000,
};
timer_t id;
remain = nthreads + 1; /* worker threads + this thread */
signal(SIGALRM, distribution_handler);
err = timer_create(CLOCK_PROCESS_CPUTIME_ID, NULL, &id);
if (err < 0) {
ksft_perror("Can't create timer");
return -1;
}
err = timer_settime(id, 0, &val, NULL);
if (err < 0) {
ksft_perror("Can't set timer");
return -1;
}
/* 1/10 seconds to ensure the leader sleeps */
usleep(10000);
for (i = 0; i < nthreads; i++) {
err = pthread_create(&threads[i], NULL, distribution_thread,
NULL);
if (err) {
ksft_print_msg("Can't create thread: %s (%d)\n",
strerror(errno), errno);
return -1;
}
}
ctd_count = 100;
if (timer_create(CLOCK_PROCESS_CPUTIME_ID, NULL, &id))
return "Can't create timer\n";
if (timer_settime(id, 0, &val, NULL))
return "Can't set timer\n";
/* Wait for all threads to receive the signal. */
while (__atomic_load_n(&remain, __ATOMIC_RELAXED));
while (ctd_count > 0 && !ctd_failed)
;
for (i = 0; i < nthreads; i++) {
err = pthread_join(threads[i], NULL);
if (err) {
ksft_print_msg("Can't join thread: %s (%d)\n",
strerror(errno), errno);
return -1;
}
}
if (timer_delete(id))
return "Can't delete timer\n";
if (timer_delete(id)) {
ksft_perror("Can't delete timer");
return -1;
}
return NULL;
}
ksft_test_result_pass("check_timer_distribution\n");
/*
* Test that only the running thread receives the timer signal.
*/
static int check_timer_distribution(void)
{
const char *errmsg;
signal(SIGALRM, ctd_sighandler);
errmsg = "Can't create thread\n";
if (pthread_create(&ctd_thread, NULL, ctd_thread_func, NULL))
goto err;
errmsg = "Can't join thread\n";
if (pthread_join(ctd_thread, (void **)&errmsg) || errmsg)
goto err;
if (!ctd_failed)
ksft_test_result_pass("check signal distribution\n");
else if (ksft_min_kernel_version(6, 3))
ksft_test_result_fail("check signal distribution\n");
else
ksft_test_result_skip("check signal distribution (old kernel)\n");
return 0;
err:
ksft_print_msg("%s", errmsg);
return -1;
}
int main(int argc, char **argv)

View File

@ -21,9 +21,6 @@
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
@ -62,45 +59,47 @@ int clear_time_state(void)
#define NUM_FREQ_OUTOFRANGE 4
#define NUM_FREQ_INVALID 2
#define SHIFTED_PPM (1 << 16)
long valid_freq[NUM_FREQ_VALID] = {
-499<<16,
-450<<16,
-400<<16,
-350<<16,
-300<<16,
-250<<16,
-200<<16,
-150<<16,
-100<<16,
-75<<16,
-50<<16,
-25<<16,
-10<<16,
-5<<16,
-1<<16,
-499 * SHIFTED_PPM,
-450 * SHIFTED_PPM,
-400 * SHIFTED_PPM,
-350 * SHIFTED_PPM,
-300 * SHIFTED_PPM,
-250 * SHIFTED_PPM,
-200 * SHIFTED_PPM,
-150 * SHIFTED_PPM,
-100 * SHIFTED_PPM,
-75 * SHIFTED_PPM,
-50 * SHIFTED_PPM,
-25 * SHIFTED_PPM,
-10 * SHIFTED_PPM,
-5 * SHIFTED_PPM,
-1 * SHIFTED_PPM,
-1000,
1<<16,
5<<16,
10<<16,
25<<16,
50<<16,
75<<16,
100<<16,
150<<16,
200<<16,
250<<16,
300<<16,
350<<16,
400<<16,
450<<16,
499<<16,
1 * SHIFTED_PPM,
5 * SHIFTED_PPM,
10 * SHIFTED_PPM,
25 * SHIFTED_PPM,
50 * SHIFTED_PPM,
75 * SHIFTED_PPM,
100 * SHIFTED_PPM,
150 * SHIFTED_PPM,
200 * SHIFTED_PPM,
250 * SHIFTED_PPM,
300 * SHIFTED_PPM,
350 * SHIFTED_PPM,
400 * SHIFTED_PPM,
450 * SHIFTED_PPM,
499 * SHIFTED_PPM,
};
long outofrange_freq[NUM_FREQ_OUTOFRANGE] = {
-1000<<16,
-550<<16,
550<<16,
1000<<16,
-1000 * SHIFTED_PPM,
-550 * SHIFTED_PPM,
550 * SHIFTED_PPM,
1000 * SHIFTED_PPM,
};
#define LONG_MAX (~0UL>>1)