Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1451289 > unrolled thread

clocksource_watchdog causing scheduling of timers every second (was [v13] support "task_isolation" mode)

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  clocksource_watchdog causing scheduling of timers every second (was  [v13] support "task_isolation" mode) Christoph Lameter <cl@linux.com> - 2016-07-27 16:00 +0200
    Re: clocksource_watchdog causing scheduling of timers every second  (was [v13] support "task_isolation" mode) Christoph Lameter <cl@linux.com> - 2016-07-27 17:30 +0200
      Re: clocksource_watchdog causing scheduling of timers every second  (was [v13] support "task_isolation" mode) Christoph Lameter <cl@linux.com> - 2016-07-27 17:40 +0200
        Re: clocksource_watchdog causing scheduling of timers every second  (was [v13] support "task_isolation" mode) Christoph Lameter <cl@linux.com> - 2016-07-27 21:00 +0200
          Re: clocksource_watchdog causing scheduling of timers every second  (was [v13] support "task_isolation" mode) Christoph Lameter <cl@linux.com> - 2016-07-27 22:00 +0200
            Re: clocksource_watchdog causing scheduling of timers every second  (was [v13] support "task_isolation" mode) Francis Giraldeau <francis.giraldeau@gmail.com> - 2016-07-29 20:40 +0200

#1451289 — clocksource_watchdog causing scheduling of timers every second (was [v13] support "task_isolation" mode)

On Mon, 25 Jul 2016, Christoph Lameter wrote:

> Guess so. I will have a look at this when I get some time again.

Ok so the problem is the clocksource_watchdog() function in
kernel/time/clocksource.c. This function is active if
CONFIG_CLOCKSOURCE_WATCHDOG is defined. It will check the timesources of
each processor for being within bounds and then reschedule itself on the
next one.

The purpose of the function seems to be to determine *if* a clocksource is
unstable. It does not mean that the clocksource *is* unstable.

The critical piece of code is this:

        /*
         * Cycle through CPUs to check if the CPUs stay synchronized
         * to each other.
         */
        next_cpu = cpumask_next(raw_smp_processor_id(), cpu_online_mask);
        if (next_cpu >= nr_cpu_ids)
                next_cpu = cpumask_first(cpu_online_mask);
        watchdog_timer.expires += WATCHDOG_INTERVAL;
        add_timer_on(&watchdog_timer, next_cpu);


Should we just cycle through the cpus that are not isolated? Otherwise we
need to have some means to check the clocksources for accuracy remotely
(probably impossible for TSC etc).

The WATCHDOG_INTERVAL is 1 second so this causes an interrupt every
second.

Note that we are running with the patch that removes the 1 HZ mininum time
tick. With an older kernel code base (redhat) we can keep the kernel quiet
for minutes. The clocksource watchdog causes timers to fire again.

[toc] | [next] | [standalone]

#1451356 — Re: clocksource_watchdog causing scheduling of timers every second (was [v13] support "task_isolation" mode)

On Wed, 27 Jul 2016, Chris Metcalf wrote:

> > Should we just cycle through the cpus that are not isolated? Otherwise we
> > need to have some means to check the clocksources for accuracy remotely
> > (probably impossible for TSC etc).
>
> That sounds like the right idea - use the housekeeping cpu mask instead of the
> cpu online mask.  Should be a straightforward patch; do you want to do that
> and test it in your configuration, and I'll include it in the next spin of the
> patch series?

Sadly housekeeping_mask is defined the following way:

static inline const struct cpumask *housekeeping_cpumask(void)
{
#ifdef CONFIG_NO_HZ_FULL
        if (tick_nohz_full_enabled())
                return housekeeping_mask;
#endif
        return cpu_possible_mask;
}

Why is it not returning cpu_online_mask?

[toc] | [prev] | [next] | [standalone]

#1451361 — Re: clocksource_watchdog causing scheduling of timers every second (was [v13] support "task_isolation" mode)

Ok here is a possible patch that explicitly checks for housekeeping cpus:

Subject: clocksource: Do not schedule watchdog on isolated or NOHZ cpus

watchdog checks can only run on housekeeping capable cpus. Otherwise
we will be generating noise that we would like to avoid on the isolated
processors.

Signed-off-by: Christoph Lameter <cl@linux.com>

Index: linux/kernel/time/clocksource.c
===================================================================
--- linux.orig/kernel/time/clocksource.c	2016-07-27 08:41:17.109862517 -0500
+++ linux/kernel/time/clocksource.c	2016-07-27 10:28:31.172447732 -0500
@@ -269,9 +269,12 @@ static void clocksource_watchdog(unsigne
 	 * Cycle through CPUs to check if the CPUs stay synchronized
 	 * to each other.
 	 */
-	next_cpu = cpumask_next(raw_smp_processor_id(), cpu_online_mask);
-	if (next_cpu >= nr_cpu_ids)
-		next_cpu = cpumask_first(cpu_online_mask);
+	do {
+		next_cpu = cpumask_next(raw_smp_processor_id(), cpu_online_mask);
+		if (next_cpu >= nr_cpu_ids)
+			next_cpu = cpumask_first(cpu_online_mask);
+	} while (!is_housekeeping_cpu(next_cpu));
+
 	watchdog_timer.expires += WATCHDOG_INTERVAL;
 	add_timer_on(&watchdog_timer, next_cpu);
 out:

[toc] | [prev] | [next] | [standalone]

#1451468 — Re: clocksource_watchdog causing scheduling of timers every second (was [v13] support "task_isolation" mode)

On Wed, 27 Jul 2016, Chris Metcalf wrote:

> How about using cpumask_next_and(raw_smp_processor_id(), cpu_online_mask,
> housekeeping_cpumask()), likewise cpumask_first_and()?  Does that work?

Ok here is V2:


Subject: clocksource: Do not schedule watchdog on isolated or NOHZ cpus V2

watchdog checks can only run on housekeeping capable cpus. Otherwise
we will be generating noise that we would like to avoid on the isolated
processors.

Signed-off-by: Christoph Lameter <cl@linux.com>

Index: linux/kernel/time/clocksource.c
===================================================================
--- linux.orig/kernel/time/clocksource.c
+++ linux/kernel/time/clocksource.c
@@ -269,9 +269,10 @@ static void clocksource_watchdog(unsigne
 	 * Cycle through CPUs to check if the CPUs stay synchronized
 	 * to each other.
 	 */
-	next_cpu = cpumask_next(raw_smp_processor_id(), cpu_online_mask);
+	next_cpu = cpumask_next_and(raw_smp_processor_id(), cpu_online_mask, housekeeping_cpumask());
 	if (next_cpu >= nr_cpu_ids)
-		next_cpu = cpumask_first(cpu_online_mask);
+		next_cpu = cpumask_first_and(cpu_online_mask, housekeeping_cpumask());
+
 	watchdog_timer.expires += WATCHDOG_INTERVAL;
 	add_timer_on(&watchdog_timer, next_cpu);
 out:

[toc] | [prev] | [next] | [standalone]

#1451486 — Re: clocksource_watchdog causing scheduling of timers every second (was [v13] support "task_isolation" mode)

On Wed, 27 Jul 2016, Chris Metcalf wrote:

> Looks good.  Did you omit the equivalent fix in clocksource_start_watchdog()
> on purpose?  For now I just took your change, but tweaked it to add the
> equivalent diff with cpumask_first_and() there.

Can the watchdog be started on an isolated cpu at all? I would expect that
the code would start a watchdog only on a housekeeping cpu.

[toc] | [prev] | [next] | [standalone]

#1452515 — Re: clocksource_watchdog causing scheduling of timers every second (was [v13] support "task_isolation" mode)

I tested this patch on 4.7 and confirm that irq_work does not occurs anymore on
the isolated cpu. Thanks!

I don't know of any utility to test the task isolation feature, so I started
one:

    https://github.com/giraldeau/taskisol

The script exp.sh runs the taskisol to test five different conditions, but some
behavior is not the one I would expect.

At startup, it does:
 - register a custom signal handler for SIGUSR1
 - sched_setaffinity() on CPU 1, which is isolated
 - mlockall(MCL_CURRENT) to prevent undesired page faults

The default strict mode is set with:

    prctl(PR_SET_TASK_ISOLATION, PR_TASK_ISOLATION_ENABLE)

And then, the syscall write() is called. From previous discussion, the SIGKILL
should be sent, but it does not occur. When instead of calling write() we force
a page fault, then the SIGKILL is correctly sent.

When instead a custom signal handler SIGUSR1:

    prctl(PR_SET_TASK_ISOLATION, PR_TASK_ISOLATION_USERSIG |
                      PR_TASK_ISOLATION_SET_SIG(SIGUSR1)

The signal is never delivered, either when the syscall is issued nor when the
page fault occurs.

I can confirm that, if two taskisol are created on the same CPU, the second one
fails with Resource temporarily unavailable, so that's fine.

I can add more test cases depending on your comments, such as the TLB events
triggered by another thread on a non-isolated core. But maybe there is already
a test suite?

Francis

2016-07-27 15:58 GMT-04:00 Chris Metcalf <cmetcalf@mellanox.com>:
> On 7/27/2016 3:53 PM, Christoph Lameter wrote:
>>
>> On Wed, 27 Jul 2016, Chris Metcalf wrote:
>>
>>> Looks good.  Did you omit the equivalent fix in
>>> clocksource_start_watchdog()
>>> on purpose?  For now I just took your change, but tweaked it to add the
>>> equivalent diff with cpumask_first_and() there.
>>
>> Can the watchdog be started on an isolated cpu at all? I would expect that
>> the code would start a watchdog only on a housekeeping cpu.
>
>
> The code just starts the watchdog initially on the first online cpu.
> In principle you could have configured that as an isolated cpu, so
> without any change to that code, you'd interrupt that cpu.
>
> I guess another way to slice it would be to start the watchdog on the
> current core.  But just using the same idiom as in clocksource_watchdog()
> seems cleanest to me.
>
> I added your patch to the series and pushed it up (along with adding your
> Tested-by to the x86 enablement commit).  It's still based on 4.6 so I'll
> need
> to rebase it once the merge window closes.
>
>
> --
> Chris Metcalf, Mellanox Technologies
> http://www.mellanox.com
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web