work_on_cpu() is not protected against CPU hotplug. For code which requires
to be either executed on an online CPU or to fail if the CPU is not
available the callsite would have to protect against CPU hotplug.
Provide a function which does get/put_online_cpus() around the call to
work_on_cpu() and fails the call with -ENODEV if the target CPU is not
online.
Preparatory patch to convert several racy task affinity manipulations.
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: Lai Jiangshan <[email protected]>
---
include/linux/workqueue.h | 5 +++++
kernel/workqueue.c | 23 +++++++++++++++++++++++
2 files changed, 28 insertions(+)
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -608,8 +608,13 @@ static inline long work_on_cpu(int cpu,
{
return fn(arg);
}
+static inline long work_on_cpu_safe(int cpu, long (*fn)(void *), void *arg)
+{
+ return fn(arg);
+}
#else
long work_on_cpu(int cpu, long (*fn)(void *), void *arg);
+long work_on_cpu_safe(int cpu, long (*fn)(void *), void *arg);
#endif /* CONFIG_SMP */
#ifdef CONFIG_FREEZER
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -4735,6 +4735,29 @@ long work_on_cpu(int cpu, long (*fn)(voi
return wfc.ret;
}
EXPORT_SYMBOL_GPL(work_on_cpu);
+
+/**
+ * work_on_cpu_safe - run a function in thread context on a particular cpu
+ * @cpu: the cpu to run on
+ * @fn: the function to run
+ * @arg: the function argument
+ *
+ * Disables CPU hotplug and calls work_on_cpu(). The caller must not hold
+ * any locks which would prevent @fn from completing.
+ *
+ * Return: The value @fn returns.
+ */
+long work_on_cpu_safe(int cpu, long (*fn)(void *), void *arg)
+{
+ long ret = -ENODEV;
+
+ get_online_cpus();
+ if (cpu_online(cpu))
+ ret = work_on_cpu(cpu, fn, arg);
+ put_online_cpus();
+ return ret;
+}
+EXPORT_SYMBOL_GPL(work_on_cpu_safe);
#endif /* CONFIG_SMP */
#ifdef CONFIG_FREEZER
At 04/13/2017 04:07 AM, Thomas Gleixner wrote:
> work_on_cpu() is not protected against CPU hotplug. For code which requires
> to be either executed on an online CPU or to fail if the CPU is not
> available the callsite would have to protect against CPU hotplug.
>
> Provide a function which does get/put_online_cpus() around the call to
> work_on_cpu() and fails the call with -ENODEV if the target CPU is not
> online.
>
> Preparatory patch to convert several racy task affinity manipulations.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: Tejun Heo <[email protected]>
> Cc: Lai Jiangshan <[email protected]>
> ---
> include/linux/workqueue.h | 5 +++++
> kernel/workqueue.c | 23 +++++++++++++++++++++++
> 2 files changed, 28 insertions(+)
>
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -608,8 +608,13 @@ static inline long work_on_cpu(int cpu,
> {
> return fn(arg);
> }
> +static inline long work_on_cpu_safe(int cpu, long (*fn)(void *), void *arg)
> +{
> + return fn(arg);
> +}
> #else
> long work_on_cpu(int cpu, long (*fn)(void *), void *arg);
> +long work_on_cpu_safe(int cpu, long (*fn)(void *), void *arg);
> #endif /* CONFIG_SMP */
>
> #ifdef CONFIG_FREEZER
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -4735,6 +4735,29 @@ long work_on_cpu(int cpu, long (*fn)(voi
> return wfc.ret;
> }
> EXPORT_SYMBOL_GPL(work_on_cpu);
> +
> +/**
There is an extra '*' :)
Thanks,
Liyang
> + * work_on_cpu_safe - run a function in thread context on a particular cpu
> + * @cpu: the cpu to run on
> + * @fn: the function to run
> + * @arg: the function argument
> + *
> + * Disables CPU hotplug and calls work_on_cpu(). The caller must not hold
> + * any locks which would prevent @fn from completing.
> + *
> + * Return: The value @fn returns.
> + */
> +long work_on_cpu_safe(int cpu, long (*fn)(void *), void *arg)
> +{
> + long ret = -ENODEV;
> +
> + get_online_cpus();
> + if (cpu_online(cpu))
> + ret = work_on_cpu(cpu, fn, arg);
> + put_online_cpus();
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(work_on_cpu_safe);
> #endif /* CONFIG_SMP */
>
> #ifdef CONFIG_FREEZER
On Thu, 13 Apr 2017, Dou Liyang wrote:
> At 04/13/2017 04:07 AM, Thomas Gleixner wrote:
> > +
> > +/**
>
> There is an extra '*' :)
On purpose. It's the begin of a kernel-doc format comment for interface
documentation.
Thanks,
tglx
On Wed, Apr 12, 2017 at 10:07:28PM +0200, Thomas Gleixner wrote:
> work_on_cpu() is not protected against CPU hotplug. For code which requires
> to be either executed on an online CPU or to fail if the CPU is not
> available the callsite would have to protect against CPU hotplug.
>
> Provide a function which does get/put_online_cpus() around the call to
> work_on_cpu() and fails the call with -ENODEV if the target CPU is not
> online.
>
> Preparatory patch to convert several racy task affinity manipulations.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: Tejun Heo <[email protected]>
> Cc: Lai Jiangshan <[email protected]>
Acked-by: Tejun Heo <[email protected]>
Thanks.
--
tejun
On Wed, Apr 12, 2017 at 10:07:28PM +0200, Thomas Gleixner wrote:
> +long work_on_cpu_safe(int cpu, long (*fn)(void *), void *arg)
> +{
> + long ret = -ENODEV;
> +
> + get_online_cpus();
> + if (cpu_online(cpu))
> + ret = work_on_cpu(cpu, fn, arg);
> + put_online_cpus();
> + return ret;
> +}
But doesn't workqueue have this lovelt 'feature' where it will unbind
per-cpu work and run it on random CPUs when hotplug happens?
That is, I think you need a flush_work() before put_online_cpus() if you
want to guarantee anything.
On Fri, 14 Apr 2017, Peter Zijlstra wrote:
> On Wed, Apr 12, 2017 at 10:07:28PM +0200, Thomas Gleixner wrote:
> > +long work_on_cpu_safe(int cpu, long (*fn)(void *), void *arg)
> > +{
> > + long ret = -ENODEV;
> > +
> > + get_online_cpus();
> > + if (cpu_online(cpu))
> > + ret = work_on_cpu(cpu, fn, arg);
> > + put_online_cpus();
> > + return ret;
> > +}
>
> But doesn't workqueue have this lovelt 'feature' where it will unbind
> per-cpu work and run it on random CPUs when hotplug happens?
>
> That is, I think you need a flush_work() before put_online_cpus() if you
> want to guarantee anything.
work_on_cpu() is sychnronous, it flushes already.
Thanks,
tglx
On Fri, Apr 14, 2017 at 11:51:07AM +0200, Thomas Gleixner wrote:
> On Fri, 14 Apr 2017, Peter Zijlstra wrote:
>
> > On Wed, Apr 12, 2017 at 10:07:28PM +0200, Thomas Gleixner wrote:
> > > +long work_on_cpu_safe(int cpu, long (*fn)(void *), void *arg)
> > > +{
> > > + long ret = -ENODEV;
> > > +
> > > + get_online_cpus();
> > > + if (cpu_online(cpu))
> > > + ret = work_on_cpu(cpu, fn, arg);
> > > + put_online_cpus();
> > > + return ret;
> > > +}
> >
> > But doesn't workqueue have this lovelt 'feature' where it will unbind
> > per-cpu work and run it on random CPUs when hotplug happens?
> >
> > That is, I think you need a flush_work() before put_online_cpus() if you
> > want to guarantee anything.
>
> work_on_cpu() is sychnronous, it flushes already.
So much for being awake ...
Commit-ID: 0e8d6a9336b487a1dd6f1991ff376e669d4c87c6
Gitweb: http://git.kernel.org/tip/0e8d6a9336b487a1dd6f1991ff376e669d4c87c6
Author: Thomas Gleixner <[email protected]>
AuthorDate: Wed, 12 Apr 2017 22:07:28 +0200
Committer: Thomas Gleixner <[email protected]>
CommitDate: Sat, 15 Apr 2017 12:20:53 +0200
workqueue: Provide work_on_cpu_safe()
work_on_cpu() is not protected against CPU hotplug. For code which requires
to be either executed on an online CPU or to fail if the CPU is not
available the callsite would have to protect against CPU hotplug.
Provide a function which does get/put_online_cpus() around the call to
work_on_cpu() and fails the call with -ENODEV if the target CPU is not
online.
Preparatory patch to convert several racy task affinity manipulations.
Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: Tejun Heo <[email protected]>
Cc: Fenghua Yu <[email protected]>
Cc: Tony Luck <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Sebastian Siewior <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Len Brown <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
include/linux/workqueue.h | 5 +++++
kernel/workqueue.c | 23 +++++++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index bde063c..c102ef6 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -608,8 +608,13 @@ static inline long work_on_cpu(int cpu, long (*fn)(void *), void *arg)
{
return fn(arg);
}
+static inline long work_on_cpu_safe(int cpu, long (*fn)(void *), void *arg)
+{
+ return fn(arg);
+}
#else
long work_on_cpu(int cpu, long (*fn)(void *), void *arg);
+long work_on_cpu_safe(int cpu, long (*fn)(void *), void *arg);
#endif /* CONFIG_SMP */
#ifdef CONFIG_FREEZER
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index c0168b7..5bf1be0 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -4735,6 +4735,29 @@ long work_on_cpu(int cpu, long (*fn)(void *), void *arg)
return wfc.ret;
}
EXPORT_SYMBOL_GPL(work_on_cpu);
+
+/**
+ * work_on_cpu_safe - run a function in thread context on a particular cpu
+ * @cpu: the cpu to run on
+ * @fn: the function to run
+ * @arg: the function argument
+ *
+ * Disables CPU hotplug and calls work_on_cpu(). The caller must not hold
+ * any locks which would prevent @fn from completing.
+ *
+ * Return: The value @fn returns.
+ */
+long work_on_cpu_safe(int cpu, long (*fn)(void *), void *arg)
+{
+ long ret = -ENODEV;
+
+ get_online_cpus();
+ if (cpu_online(cpu))
+ ret = work_on_cpu(cpu, fn, arg);
+ put_online_cpus();
+ return ret;
+}
+EXPORT_SYMBOL_GPL(work_on_cpu_safe);
#endif /* CONFIG_SMP */
#ifdef CONFIG_FREEZER