| 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
| ||
|
Date: Thu, 25 Jun 2015 15:55:56 +0200 (CEST)
From: Thomas Gleixner <tglx@...utronix.de>
To: Sudeep Holla <sudeep.holla@....com>
cc: linux-kernel@...r.kernel.org,
Suzuki Poulose <Suzuki.Poulose@....com>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Catalin Marinas <catalin.marinas@....com>,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
Preeti U Murthy <preeti@...ux.vnet.ibm.com>
Subject: Re: [PATCH] clockevents: return error from tick_broadcast_oneshot_control
if !GENERIC_CLOCKEVENTS_BROADCAST
On Thu, 25 Jun 2015, Sudeep Holla wrote:
> tick_broadcast_enter returns 0 when CPU can switch to broadcast
> timer and non-zero otherwise. However when GENERIC_CLOCKEVENTS_BROADCAST
> and TICK_ONESHOT are disabled, tick_broadcast_oneshot_control returns 0
> which indicates to the CPUIdle framework that the CPU can enter deeper
> idle states even when the CPU local timer will be shutdown. If the
> target state needs broadcast but not broadcast timer is available, then
> the CPU can not resume back from that idle state.
>
> This patch returns error when there's no broadcast timer support
> available so that CPUIdle framework prevents the CPU from entering any
> idle states losing the local timer.
That's wrong and breaks stuff which does not require the broadcast
nonsense.
If TICK_ONESHOT is disabled, then everything is in periodic mode and
tick_broadcast_enter() rightfully returns 0. Ditto for 'highres=off'
on the command line.
But there is a case which is not correctly handled right now. That's
what you are trying to solve in the wrong way.
If
GENERIC_CLOCKEVENTS_BROADCAST=n
or
GENERIC_CLOCKEVENTS_BROADCAST=y and no broadcast device is available,
AND cpu local tick device has the C3STOP flag set,
then we have no way to tell the idle code that going deep is not
allowed.
So we need to be smarter than blindly changing a return
value. Completely untested patch below.
Thanks,
tglx
---
diff --git a/include/linux/tick.h b/include/linux/tick.h
index 4191b5623a28..8731a58dd747 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -63,11 +63,7 @@ extern void tick_broadcast_control(enum tick_broadcast_mode mode);
static inline void tick_broadcast_control(enum tick_broadcast_mode mode) { }
#endif /* BROADCAST */
-#if defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) && defined(CONFIG_TICK_ONESHOT)
extern int tick_broadcast_oneshot_control(enum tick_broadcast_state state);
-#else
-static inline int tick_broadcast_oneshot_control(enum tick_broadcast_state state) { return 0; }
-#endif
static inline void tick_broadcast_enable(void)
{
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index d39f32cdd1b5..52c8d01b956b 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -662,24 +662,15 @@ static void broadcast_shutdown_local(struct clock_event_device *bc,
clockevents_switch_state(dev, CLOCK_EVT_STATE_SHUTDOWN);
}
-/**
- * tick_broadcast_oneshot_control - Enter/exit broadcast oneshot mode
- * @state: The target state (enter/exit)
- *
- * The system enters/leaves a state, where affected devices might stop
- * Returns 0 on success, -EBUSY if the cpu is used to broadcast wakeups.
- *
- * Called with interrupts disabled, so clockevents_lock is not
- * required here because the local clock event device cannot go away
- * under us.
- */
-int tick_broadcast_oneshot_control(enum tick_broadcast_state state)
+int __tick_broadcast_oneshot_control(enum tick_broadcast_state state)
{
struct clock_event_device *bc, *dev;
- struct tick_device *td;
int cpu, ret = 0;
ktime_t now;
+ if (!tick_broadcast_device.evtdev)
+ return -EBUSY;
+
/*
* Periodic mode does not care about the enter/exit of power
* states
@@ -687,15 +678,7 @@ int tick_broadcast_oneshot_control(enum tick_broadcast_state state)
if (tick_broadcast_device.mode == TICKDEV_MODE_PERIODIC)
return 0;
- /*
- * We are called with preemtion disabled from the depth of the
- * idle code, so we can't be moved away.
- */
- td = this_cpu_ptr(&tick_cpu_device);
- dev = td->evtdev;
-
- if (!(dev->features & CLOCK_EVT_FEAT_C3STOP))
- return 0;
+ dev = this_cpu_ptr(&tick_cpu_device)->evtdev;
raw_spin_lock(&tick_broadcast_lock);
bc = tick_broadcast_device.evtdev;
@@ -938,6 +921,13 @@ bool tick_broadcast_oneshot_available(void)
return bc ? bc->features & CLOCK_EVT_FEAT_ONESHOT : false;
}
+#else
+int __tick_broadcast_oneshot_control(enum tick_broadcast_state state)
+{
+ if (!tick_broadcast_device.evtdev)
+ return -EBUSY;
+ return 0;
+}
#endif
void __init tick_broadcast_init(void)
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 17f144450050..f66c11a30348 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -342,6 +342,27 @@ out_bc:
tick_install_broadcast_device(newdev);
}
+/**
+ * tick_broadcast_oneshot_control - Enter/exit broadcast oneshot mode
+ * @state: The target state (enter/exit)
+ *
+ * The system enters/leaves a state, where affected devices might stop
+ * Returns 0 on success, -EBUSY if the cpu is used to broadcast wakeups.
+ *
+ * Called with interrupts disabled, so clockevents_lock is not
+ * required here because the local clock event device cannot go away
+ * under us.
+ */
+int tick_broadcast_oneshot_control(enum tick_broadcast_state state)
+{
+ struct tick_device *td = this_cpu_ptr(&tick_cpu_device);
+
+ if (!(td->evtdev->features & CLOCK_EVT_FEAT_C3STOP))
+ return 0;
+
+ return __tick_broadcast_oneshot_control(state);
+}
+
#ifdef CONFIG_HOTPLUG_CPU
/*
* Transfer the do_timer job away from a dying cpu.
diff --git a/kernel/time/tick-sched.h b/kernel/time/tick-sched.h
index 42fdf4958bcc..a4a8d4e9baa1 100644
--- a/kernel/time/tick-sched.h
+++ b/kernel/time/tick-sched.h
@@ -71,4 +71,14 @@ extern void tick_cancel_sched_timer(int cpu);
static inline void tick_cancel_sched_timer(int cpu) { }
#endif
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
+extern int __tick_broadcast_oneshot_control(enum tick_broadcast_state state);
+#else
+static inline int
+__tick_broadcast_oneshot_control(enum tick_broadcast_state state)
+{
+ return -EBUSY;
+}
+#endif
+
#endif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists