linux-kernel - [RFC][PATCH] irq/timings: Ignore predictions in the past

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181107100908.GJ9781@hirez.programming.kicks-ass.net>
Date:   Wed, 7 Nov 2018 11:09:08 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Linux PM <linux-pm@...r.kernel.org>,
        Giovanni Gherdovich <ggherdovich@...e.cz>,
        Doug Smythies <dsmythies@...us.net>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Frederic Weisbecker <frederic@...nel.org>,
        Mel Gorman <mgorman@...e.de>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Nicolas Pitre <nicolas.pitre@...aro.org>
Subject: [RFC][PATCH] irq/timings: Ignore predictions in the past

On Wed, Nov 07, 2018 at 12:39:31AM +0100, Rafael J. Wysocki wrote:
> In general, however, I need to be convinced that interrupts that
> didn't wake up the CPU from idle are relevant for next wakeup
> prediction.

So you're worried about the case where we're 100% busy and generating IO
interrupts (disk, network, etc..) and then go idle and those interrupts
stop happening, but we'll base a prediction off of them?

Any predictor will have some of those trancients, but note that we'll
only actually train the irq timing thing when here is idle time. If
we're 100% busy, we'll never actually consume the data.

Also, I think the below delta (on top of my earlier patch) would make
sense; as I don't see how the current code makes sense there.

--- a/kernel/irq/timings.c
+++ b/kernel/irq/timings.c
@@ -306,22 +306,17 @@ u64 irq_timings_next_event(u64 now)
 			continue;
 
 		if (irqs->next_evt <= now) {
-			irq = i;
-			next_evt = now;
-
 			/*
-			 * This interrupt mustn't use in the future
-			 * until new events occur and update the
-			 * statistics.
+			 * If the last prediction was in the past, mark the IRQ
+			 * as invalid until a next interrupt updates the
+			 * prediction.
 			 */
 			irqs->valid = 0;
-			break;
+			continue;
 		}
 
-		if (irqs->next_evt < next_evt) {
-			irq = i;
+		if (irqs->next_evt < next_evt)
 			next_evt = irqs->next_evt;
-		}
 	}
 
 	return next_evt;