From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:57:28
My intention is to make it easier to manipulate and maintain kthreads.
Especially, I want to replace all the custom main cycles with a
generic one. Also I want to make the kthreads sleep in a consistent
state in a common place when there is no work.
My first attempt was with a brand new API (iterant kthread), see
http://thread.gmane.org/gmane.linux.kernel.api/11892 . But I was
directed to improve the existing kthread worker API. This is
the 4th iteration of the new direction.
1nd..10th patches: improve the existing kthread worker API
11th..16th, 18th, 20th patches: convert several kthreads into
the kthread worker API, namely: khugepaged, ring buffer
benchmark, hung_task, kmemleak, ipmi, IB/fmr_pool,
memstick/r592, intel_powerclamp
17th, 19th patches: do some preparation steps; they usually do
some clean up that makes sense even without the conversion.
Changes against v4:
+ added worker->delayed_work_list; it simplified the check
for pending work; we do not longer need the new timer_active()
function; also we do not need the link work->timer. On the
other hand we need to distinguish between the normal and
the delayed work by a boolean parameter passed to
the common functions, e.g. __cancel_kthread_work_sync()
+ replaced most try_lock repeat cycles with a WARN_ON();
the API does not allow to use the work with more workers;
so such a situation would be a bug; it removed the
complex try_lock_kthread_work() function that supported
more modes;
+ renamed kthread_work_pending() to queuing_blocked();
added this function later when really needed
+ renamed try_to_cancel_kthread_work() to __cancel_kthread_work();
in fact, this a common implementation for the async cancel()
function
+ removed a dull check for invalid cpu number in
create_kthread_worker_on_cpu(); removed some other unnecessary
code structures as suggested by Tejun
+ consistently used bool return value in all new __cancel functions
+ fixed ordering of cpu and flags parameters in
create_kthread_worker_on_cpu() vs. create_kthread_worker()
+ used memset in the init_kthread_worker()
+ updated many comments as suggested by Tejun and as
required the above changes
+ removed obsolete patch adding timer_active()
+ removed obsolete patch for using try_lock in flush_kthread_worker()
+ double checked all existing users of kthread worker API
that they reinitialized the work when the worker was started
and would not print false warnings; all looked fine
+ added taken acks for the Intel Powerclamp conversion
Changes against v3:
+ allow to free struct kthread_work from its callback; do not touch
the struct from the worker post-mortem; as a side effect, the structure
must be reinitialized when the worker gets restarted; updated
khugepaged, and kmemleak accordingly
+ call del_timer_sync() with worker->lock; instead, detect canceling
in the timer callback and give up an attempt to get the lock there;
do busy loop with spin_is_locked() to reduce cache bouncing
+ renamed ipmi+func() -> ipmi_kthread_worker_func() as suggested
by Corey
+ added some collected Reviewed-by
Changes against v2:
+ used worker->lock to synchronize the operations with the work
instead of the PENDING bit as suggested by Tejun Heo; it simplified
the implementation in several ways
+ added timer_active(); used it together with del_timer_sync()
to cancel the work a less tricky way
+ removed the controversial conversion of the RCU kthreads
+ added several other examples: hung_task, kmemleak, ipmi,
IB/fmr_pool, memstick/r592, intel_powerclamp
+ the helper fixes for the ring buffer benchmark has been improved
as suggested by Steven; they already are in the Linus tree now
+ fixed a possible race between the check for existing khugepaged
worker and queuing the work
Changes against v1:
+ remove wrappers to manipulate the scheduling policy and priority
+ remove questionable wakeup_and_destroy_kthread_worker() variant
+ do not check for chained work when draining the queue
+ allocate struct kthread worker in create_kthread_work() and
use more simple checks for running worker
+ add support for delayed kthread works and use them instead
of waiting inside the works
+ rework the "unrelated" fixes for the ring buffer benchmark
as discussed in the 1st RFC; also sent separately
+ convert also the consumer in the ring buffer benchmark
I have tested this patch set against the stable Linus tree
for 4.5-rc5.
Comments against v3 can be found at
http://thread.gmane.org/gmane.linux.kernel.mm/144964
Petr Mladek (20):
kthread/smpboot: Do not park in kthread_create_on_cpu()
kthread: Allow to call __kthread_create_on_node() with va_list args
kthread: Add create_kthread_worker*()
kthread: Add drain_kthread_worker()
kthread: Add destroy_kthread_worker()
kthread: Detect when a kthread work is used by more workers
kthread: Initial support for delayed kthread work
kthread: Allow to cancel kthread work
kthread: Allow to modify delayed kthread work
kthread: Better support freezable kthread workers
mm/huge_page: Convert khugepaged() into kthread worker API
ring_buffer: Convert benchmark kthreads into kthread worker API
hung_task: Convert hungtaskd into kthread worker API
kmemleak: Convert kmemleak kthread into kthread worker API
ipmi: Convert kipmi kthread into kthread worker API
IB/fmr_pool: Convert the cleanup thread into kthread worker API
memstick/r592: Better synchronize debug messages in r592_io kthread
memstick/r592: convert r592_io kthread into kthread worker API
thermal/intel_powerclamp: Remove duplicated code that starts the
kthread
thermal/intel_powerclamp: Convert the kthread to kthread worker API
drivers/char/ipmi/ipmi_si_intf.c | 121 ++++----
drivers/infiniband/core/fmr_pool.c | 54 ++--
drivers/memstick/host/r592.c | 61 ++--
drivers/memstick/host/r592.h | 5 +-
drivers/thermal/intel_powerclamp.c | 302 +++++++++++--------
include/linux/kthread.h | 56 ++++
kernel/hung_task.c | 41 ++-
kernel/kthread.c | 555 +++++++++++++++++++++++++++++++----
kernel/smpboot.c | 5 +
kernel/trace/ring_buffer_benchmark.c | 133 ++++-----
mm/huge_memory.c | 138 +++++----
mm/kmemleak.c | 87 +++---
12 files changed, 1064 insertions(+), 494 deletions(-)
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: linux-watchdog@vger.kernel.org
CC: Corey Minyard <redacted>
CC: openipmi-developer@lists.sourceforge.net
CC: Doug Ledford <redacted>
CC: Sean Hefty <redacted>
CC: Hal Rosenstock <redacted>
CC: linux-rdma@vger.kernel.org
CC: Maxim Levitsky <maximlevitsky@gmail.com>
CC: Zhang Rui <rui.zhang@intel.com>
CC: Eduardo Valentin <edubezval@gmail.com>
CC: Jacob Pan <redacted>
CC: linux-pm@vger.kernel.org
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:57:26
kthread_create_on_cpu() was added by the commit 2a1d446019f9a5983e
("kthread: Implement park/unpark facility"). It is currently used
only when enabling new CPU. For this purpose, the newly created
kthread has to be parked.
The CPU binding is a bit tricky. The kthread is parked when the CPU
has not been allowed yet. And the CPU is bound when the kthread
is unparked.
The function would be useful for more per-CPU kthreads, e.g.
bnx2fc_thread, fcoethread. For this purpose, the newly created
kthread should stay in the uninterruptible state.
This patch moves the parking into smpboot. It binds the thread
already when created. Then the function might be used universally.
Also the behavior is consistent with kthread_create() and
kthread_create_on_node().
Signed-off-by: Petr Mladek <pmladek-IBi9RG/b67k@public.gmane.org>
Reviewed-by: Thomas Gleixner <redacted>
---
kernel/kthread.c | 8 ++++++--
kernel/smpboot.c | 5 +++++
2 files changed, 11 insertions(+), 2 deletions(-)
@@ -390,10 +390,10 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data),cpu);if(IS_ERR(p))returnp;+kthread_bind(p,cpu);+/* CPU hotplug need to bind once again when unparking the thread. */set_bit(KTHREAD_IS_PER_CPU,&to_kthread(p)->flags);to_kthread(p)->cpu=cpu;-/* Park the thread to get it out of TASK_UNINTERRUPTIBLE state */-kthread_park(p);returnp;}
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:57:30
kthread_create_on_node() implements a bunch of logic to create
the kthread. It is already called by kthread_create_on_cpu().
We are going to extend the kthread worker API and will
need to call kthread_create_on_node() with va_list args there.
This patch does only a refactoring and does not modify the existing
behavior.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kthread.c | 72 +++++++++++++++++++++++++++++++++-----------------------
1 file changed, 42 insertions(+), 30 deletions(-)
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:57:36
Kthread workers are currently created using the classic kthread API,
namely kthread_run(). kthread_worker_fn() is passed as the @threadfn
parameter.
This patch defines create_kthread_worker() and
create_kthread_worker_on_cpu() functions that hide implementation details.
They enforce using kthread_worker_fn() for the main thread. But I doubt
that there are any plans to create any alternative. In fact, I think
that we do not want any alternative main thread because it would be
hard to support consistency with the rest of the kthread worker API.
The naming and function is inspired by the workqueues API like the rest
of the kthread worker API.
Note that we need to bind per-CPU kthread workers already when they are
created. It makes the life easier. kthread_bind() could not be used later
for an already running worker.
This patch does _not_ convert existing kthread workers. The kthread worker
API need more improvements first, e.g. a function to destroy the worker.
Signed-off-by: Petr Mladek <pmladek-IBi9RG/b67k@public.gmane.org>
---
include/linux/kthread.h | 7 ++++
kernel/kthread.c | 95 +++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 92 insertions(+), 10 deletions(-)
kernel/kthread.c:691:9: error: incompatible type for argument 3 of '__create_kthread_worker'
return __create_kthread_worker(cpu, namefmt, NULL);
^
kernel/kthread.c:622:1: note: expected 'va_list' but argument is of type 'void *'
__create_kthread_worker(int cpu, const char namefmt[], va_list args)
^
quoted
kernel/kthread.c:692:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
vim +/__create_kthread_worker +691 kernel/kthread.c
685 * when the needed structures could not get allocated, and ERR_PTR(-EINTR)
686 * when the worker was SIGKILLed.
687 */
688 struct kthread_worker *
689 create_kthread_worker_on_cpu(int cpu, const char namefmt[])
690 {
> 691 return __create_kthread_worker(cpu, namefmt, NULL);
> 692 }
693 EXPORT_SYMBOL(create_kthread_worker_on_cpu);
694
695 /* insert @work before @pos in @worker */
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
kernel/kthread.c:691:9: error: incompatible type for argument 3 of '__create_kthread_worker'
return __create_kthread_worker(cpu, namefmt, NULL);
^
kernel/kthread.c:622:1: note: expected 'va_list' but argument is of type 'void *'
__create_kthread_worker(int cpu, const char namefmt[], va_list args)
^
quoted
quoted
kernel/kthread.c:692:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
vim +/__create_kthread_worker +691 kernel/kthread.c
685 * when the needed structures could not get allocated, and ERR_PTR(-EINTR)
686 * when the worker was SIGKILLed.
687 */
688 struct kthread_worker *
689 create_kthread_worker_on_cpu(int cpu, const char namefmt[])
690 {
> 691 return __create_kthread_worker(cpu, namefmt, NULL);
> 692 }
693 EXPORT_SYMBOL(create_kthread_worker_on_cpu);
694
695 /* insert @work before @pos in @worker */
I can be fixed by passing a fake va_list. It is not used when
__create_kthread_worker() is called with a valid CPU number.
See below an updated patch that passes the build.
From 5e6c252921c0370a88d2ce5f61370fef573afcaa Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek@suse.com>
Date: Tue, 23 Jun 2015 17:09:19 +0200
Subject: [PATCH 3/3] kthread: Add create_kthread_worker*()
Kthread workers are currently created using the classic kthread API,
namely kthread_run(). kthread_worker_fn() is passed as the @threadfn
parameter.
This patch defines create_kthread_worker() and
create_kthread_worker_on_cpu() functions that hide implementation details.
They enforce using kthread_worker_fn() for the main thread. But I doubt
that there are any plans to create any alternative. In fact, I think
that we do not want any alternative main thread because it would be
hard to support consistency with the rest of the kthread worker API.
The naming and function is inspired by the workqueues API like the rest
of the kthread worker API.
Note that we need to bind per-CPU kthread workers already when they are
created. It makes the life easier. kthread_bind() could not be used later
for an already running worker.
This patch does _not_ convert existing kthread workers. The kthread worker
API need more improvements first, e.g. a function to destroy the worker.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 7 ++++
kernel/kthread.c | 97 ++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 94 insertions(+), 10 deletions(-)
@@ -617,6 +618,82 @@ repeat:}EXPORT_SYMBOL_GPL(kthread_worker_fn);+staticstructkthread_worker*+__create_kthread_worker(intcpu,constcharnamefmt[],va_listargs)+{+structkthread_worker*worker;+structtask_struct*task;++worker=kzalloc(sizeof(*worker),GFP_KERNEL);+if(!worker)+returnERR_PTR(-ENOMEM);++init_kthread_worker(worker);++if(cpu>=0)+task=kthread_create_on_cpu(kthread_worker_fn,worker,+cpu,namefmt);+else+task=__kthread_create_on_node(kthread_worker_fn,worker,+-1,namefmt,args);+if(IS_ERR(task))+gotofail_task;++worker->task=task;+wake_up_process(task);+returnworker;++fail_task:+kfree(worker);+returnERR_CAST(task);+}++/**+*create_kthread_worker-createakthreadworker+*@namefmt:printf-stylenameforthekthreadworker(task).+*+*Returnsapointertotheallocatedworkeronsuccess,ERR_PTR(-ENOMEM)+*whentheneededstructurescouldnotgetallocated,andERR_PTR(-EINTR)+*whentheworkerwasSIGKILLed.+*/+structkthread_worker*+create_kthread_worker(constcharnamefmt[],...)+{+structkthread_worker*worker;+va_listargs;++va_start(args,namefmt);+worker=__create_kthread_worker(-1,namefmt,args);+va_end(args);++returnworker;+}+EXPORT_SYMBOL(create_kthread_worker);++/**+*create_kthread_worker_on_cpu-createakthreadworkerandbindit+*ittoagivenCPUandtheassociatedNUMAnode.+*@cpu:CPUnumber+*@namefmt:printf-stylenameforthekthreadworker(task).+*+*UseavalidCPUnumberifyouwanttobindthekthreadworker+*tothegivenCPUandtheassociatedNUMAnode.+*+*@namefmtmightincludeone"%d"thatwillgetreplacedbyCPUnumber.+*+*Returnsapointertotheallocatedworkeronsuccess,ERR_PTR(-ENOMEM)+*whentheneededstructurescouldnotgetallocated,andERR_PTR(-EINTR)+*whentheworkerwasSIGKILLed.+*/+structkthread_worker*+create_kthread_worker_on_cpu(intcpu,constcharnamefmt[])+{+va_listfake_args;++return__create_kthread_worker(cpu,namefmt,fake_args);+}+EXPORT_SYMBOL(create_kthread_worker_on_cpu);+/* insert @work before @pos in @worker */staticvoidinsert_kthread_work(structkthread_worker*worker,structkthread_work*work,
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:57:39
flush_kthread_worker() returns when the currently queued works are proceed.
But some other works might have been queued in the meantime.
This patch adds drain_kthread_worker() that is inspired by
drain_workqueue(). It returns when the queue is completely
empty and warns when it takes too long.
The initial implementation does not block queuing new works when
draining. It makes things much easier. The blocking would be useful
to debug potential problems but it is not clear if it is worth
the complication at the moment.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kthread.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
From: Peter Zijlstra <peterz@infradead.org> Date: 2016-02-25 12:36:04
On Mon, Feb 22, 2016 at 03:56:54PM +0100, Petr Mladek wrote:
+/**
+ * drain_kthread_worker - drain a kthread worker
+ * @worker: worker to be drained
+ *
+ * Wait until there is no work queued for the given kthread worker.
+ * @worker is flushed repeatedly until it becomes empty. The number
+ * of flushing is determined by the depth of chaining and should
+ * be relatively short. Whine if it takes too long.
+ *
+ * The caller is responsible for blocking all users of this kthread
+ * worker from queuing new works. Also it is responsible for blocking
+ * the already queued works from an infinite re-queuing!
+ */
+void drain_kthread_worker(struct kthread_worker *worker)
+{
+ int flush_cnt = 0;
+
+ spin_lock_irq(&worker->lock);
Would it not make sense to set a flag here that inhibits (or warns)
queueing new work?
Otherwise this can, as you point out, last forever.
And I think its a logic fail if you both want to drain it and keeping
adding new work.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-26 15:23:13
On Thu 2016-02-25 13:35:51, Peter Zijlstra wrote:
On Mon, Feb 22, 2016 at 03:56:54PM +0100, Petr Mladek wrote:
quoted
+/**
+ * drain_kthread_worker - drain a kthread worker
+ * @worker: worker to be drained
+ *
+ * Wait until there is no work queued for the given kthread worker.
+ * @worker is flushed repeatedly until it becomes empty. The number
+ * of flushing is determined by the depth of chaining and should
+ * be relatively short. Whine if it takes too long.
+ *
+ * The caller is responsible for blocking all users of this kthread
+ * worker from queuing new works. Also it is responsible for blocking
+ * the already queued works from an infinite re-queuing!
+ */
+void drain_kthread_worker(struct kthread_worker *worker)
+{
+ int flush_cnt = 0;
+
+ spin_lock_irq(&worker->lock);
Would it not make sense to set a flag here that inhibits (or warns)
queueing new work?
Otherwise this can, as you point out, last forever.
And I think its a logic fail if you both want to drain it and keeping
adding new work.
We must allow self-queuing because it might be needed to finish
the processing. We would need to detect it. Tejun suggested
to avoid this and make the code simple.
I do not have a strong opinion here. On one hand, such a check might
help with debugging. On the other hand, workqueues have happily lived
without it for years.
Thanks a lot for review,
Petr
From: Peter Zijlstra <peterz@infradead.org> Date: 2016-02-27 15:18:37
On Fri, Feb 26, 2016 at 04:23:09PM +0100, Petr Mladek wrote:
I do not have a strong opinion here. On one hand, such a check might
help with debugging. On the other hand, workqueues have happily lived
without it for years.
TJ and me have a different view on these things. I'm always for the
strictest possible semantics with strong validation. TJ always worries a
lot about existing users.
Luckily, you don't have users yet :-)
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:57:47
The current kthread worker users call flush() and stop() explicitly.
This function drains the worker, stops it, and frees the kthread_worker
struct in one call.
It is supposed to be used together with create_kthread_worker*() that
allocates struct kthread_worker.
Also note that drain() correctly handles self-queuing works in compare
with flush().
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 2 ++
kernel/kthread.c | 21 +++++++++++++++++++++
2 files changed, 23 insertions(+)
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-26 15:15:13
On Thu 2016-02-25 13:36:41, Peter Zijlstra wrote:
On Mon, Feb 22, 2016 at 03:56:55PM +0100, Petr Mladek wrote:
quoted
Also note that drain() correctly handles self-queuing works in compare
with flush().
Nothing seems to prevent adding more work after drain() observes
list_empty().
You might want to drain() more times during the kthread worker life
time to make sure that the work is done.
The user is responsible for stopping any queuing when this function
is called. The user usually needs to handle this anyway because
producing a work that could not be queued would cause problems.
To be honest, I wanted to keep the main principles of the API
compatible with workqueues. It should reduce some potential confusion.
Also it will make it easier to convert between the two APIs.
IMHO, there are work loads when you are not sure if you will
need a dedicated kthread when designing a new functionality.
Best Regards,
Petr
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:57:58
Nothing currently prevents a work from queuing for a kthread worker
when it is already running on another one. This means that the work
might run in parallel on more workers. Also some operations, e.g.
flush or drain are not reliable.
This problem will be even more visible after we add cancel_kthread_work()
function. It will only have "work" as the parameter and will use
worker->lock to synchronize with others.
Well, normally this is not a problem because the API users are sane.
But bugs might happen and users also might be crazy.
This patch adds a warning when we try to insert the work for another
worker. It does not fully prevent the misuse because it would make the
code much more complicated without a big benefit.
It adds the same warning also into flush_kthread_work() instead of
the repeated attempts to get the right lock.
A side effect is that one needs to explicitly reinitialize the work
if it must be queued into another worker. This is needed, for example,
when the worker is stopped and started again. It is a bit inconvenient.
But it looks like a good compromise between the stability and complexity.
I have double checked all existing users of the kthread worker API
and they all seems to initialize the work after the worker gets
started.
Just for completeness, the patch adds a check that the work is not
already in a queue.
The patch also puts all the checks into a separate function. It will
be reused when implementing delayed works.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kthread.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
@@ -692,12 +695,21 @@ create_kthread_worker_on_cpu(int cpu, const char namefmt[])}EXPORT_SYMBOL(create_kthread_worker_on_cpu);+staticvoidinsert_kthread_work_sanity_check(structkthread_worker*worker,+structkthread_work*work)+{+lockdep_assert_held(&worker->lock);+WARN_ON_ONCE(!list_empty(&work->node));+/* Do not use a work with more workers, see queue_kthread_work() */+WARN_ON_ONCE(work->worker&&work->worker!=worker);+}+/* insert @work before @pos in @worker */staticvoidinsert_kthread_work(structkthread_worker*worker,-structkthread_work*work,-structlist_head*pos)+structkthread_work*work,+structlist_head*pos){-lockdep_assert_held(&worker->lock);+insert_kthread_work_sanity_check(worker,work);list_add_tail(&work->node,pos);work->worker=worker;
@@ -757,16 +772,13 @@ void flush_kthread_work(struct kthread_work *work)structkthread_worker*worker;boolnoop=false;-retry:worker=work->worker;if(!worker)return;spin_lock_irq(&worker->lock);-if(work->worker!=worker){-spin_unlock_irq(&worker->lock);-gotoretry;-}+/* Work must not be used with more workers, see queue_kthread_work(). */+WARN_ON_ONCE(work->worker!=worker);if(!list_empty(&work->node))insert_kthread_work(worker,&fwork.work,work->node.next);
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:58:06
We are going to use kthread_worker more widely and delayed works
will be pretty useful.
The implementation is inspired by workqueues. It uses a timer to
queue the work after the requested delay. If the delay is zero,
the work is queued immediately.
In compare with workqueues, each work is associated with a single
worker (kthread). Therefore the implementation could be much easier.
In particular, we use the worker->lock to synchronize all the
operations with the work. We do not need any atomic operation
with a flags variable.
In fact, we do not need any state variable at all. Instead, we
add a list of delayed works into the worker. Then the pending
work is listed either in the list of queued or delayed works.
And the existing check of pending works is the same even for
the delayed ones.
A work must not be assigned to another worker unless reinitialized.
Therefore the timer handler might expect that dwork->work->worker
is valid and it could simply take the lock. We just add some
sanity checks to help with debugging a potential misuse.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 33 ++++++++++++++++
kernel/kthread.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 135 insertions(+)
@@ -745,6 +746,107 @@ bool queue_kthread_work(struct kthread_worker *worker,}EXPORT_SYMBOL_GPL(queue_kthread_work);+/**+*delayed_kthread_work_timer_fn-callbackthatqueuestheassociateddelayed+*kthreadworkwhenthetimerexpires.+*@__data:pointertothedataassociatedwiththetimer+*+*Theformatofthefunctionisdefinedbystructtimer_list.+*Itshouldhavebeencalledfromirqsafetimerwithirqalreadyoff.+*/+voiddelayed_kthread_work_timer_fn(unsignedlong__data)+{+structdelayed_kthread_work*dwork=+(structdelayed_kthread_work*)__data;+structkthread_work*work=&dwork->work;+structkthread_worker*worker=work->worker;++/*+*Thismighthappenwhenapendingworkisreinitialized.+*Itmeansthatitisusedawrongway.+*/+if(WARN_ON_ONCE(!worker))+return;++spin_lock(&worker->lock);+/* Work must not be used with more workers, see queue_kthread_work(). */+WARN_ON_ONCE(work->worker!=worker);++/* Move the work from worker->delayed_work_list. */+WARN_ON_ONCE(list_empty(&work->node));+list_del_init(&work->node);+insert_kthread_work(worker,work,&worker->work_list);++spin_unlock(&worker->lock);+}+EXPORT_SYMBOL(delayed_kthread_work_timer_fn);++void__queue_delayed_kthread_work(structkthread_worker*worker,+structdelayed_kthread_work*dwork,+unsignedlongdelay)+{+structtimer_list*timer=&dwork->timer;+structkthread_work*work=&dwork->work;++WARN_ON_ONCE(timer->function!=delayed_kthread_work_timer_fn||+timer->data!=(unsignedlong)dwork);++/*+*If@delayis0,queue@dwork->workimmediately.Thisisfor+*bothoptimizationandcorrectness.Theearliest@timercan+*expireisontheclosestnexttickanddelayed_workusersdepend+*onthatthere'snosuchdelaywhen@delayis0.+*/+if(!delay){+insert_kthread_work(worker,work,&worker->work_list);+return;+}++/* Be paranoid and try to detect possible races already now. */+insert_kthread_work_sanity_check(worker,work);++list_add(&work->node,&worker->delayed_work_list);+work->worker=worker;+timer_stats_timer_set_start_info(&dwork->timer);+timer->expires=jiffies+delay;+add_timer(timer);+}++/**+*queue_delayed_kthread_work-queuetheassociatedkthreadwork+*afteradelay.+*@worker:targetkthread_worker+*@work:kthread_worktoqueue+*delay:numberofjiffiestowaitbeforequeuing+*+*Iftheworkhasnotbeenpendingitstartsatimerthatwillqueue+*theworkafterthegiven@delay.If@delayiszero,itqueuesthe+*workimmediately.+*+*Return:%falseifthe@workhasalreadybeenpending.Itmeansthat+*eitherthetimerwasrunningortheworkwasqueued.Itreturns%true+*otherwise.+*/+boolqueue_delayed_kthread_work(structkthread_worker*worker,+structdelayed_kthread_work*dwork,+unsignedlongdelay)+{+structkthread_work*work=&dwork->work;+unsignedlongflags;+boolret=false;++spin_lock_irqsave(&worker->lock,flags);++if(list_empty(&work->node)){+__queue_delayed_kthread_work(worker,dwork,delay);+ret=true;+}++spin_unlock_irqrestore(&worker->lock,flags);+returnret;+}+EXPORT_SYMBOL_GPL(queue_delayed_kthread_work);+structkthread_flush_work{structkthread_workwork;structcompletiondone;
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
kernel/kthread.c:833: warning: No description found for parameter 'dwork'
kernel/kthread.c:833: warning: No description found for parameter 'delay'
kernel/kthread.c:833: warning: Excess function parameter 'work' description in 'queue_delayed_kthread_work'
kernel/sys.c:1: warning: no structured comments found
drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
drivers/dma-buf/reservation.c:1: warning: no structured comments found
include/linux/reservation.h:1: warning: no structured comments found
include/linux/spi/spi.h:540: warning: No description found for parameter 'max_transfer_size'
vim +/dwork +833 kernel/kthread.c
817 * after a delay.
818 * @worker: target kthread_worker
819 * @work: kthread_work to queue
820 * delay: number of jiffies to wait before queuing
821 *
822 * If the work has not been pending it starts a timer that will queue
823 * the work after the given @delay. If @delay is zero, it queues the
824 * work immediately.
825 *
826 * Return: %false if the @work has already been pending. It means that
827 * either the timer was running or the work was queued. It returns %true
828 * otherwise.
829 */
830 bool queue_delayed_kthread_work(struct kthread_worker *worker,
831 struct delayed_kthread_work *dwork,
832 unsigned long delay)
> 833 {
834 struct kthread_work *work = &dwork->work;
835 unsigned long flags;
836 bool ret = false;
837
838 spin_lock_irqsave(&worker->lock, flags);
839
840 if (list_empty(&work->node)) {
841 __queue_delayed_kthread_work(worker, dwork, delay);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
kernel/kthread.c:833: warning: No description found for parameter 'dwork'
kernel/kthread.c:833: warning: No description found for parameter 'delay'
kernel/kthread.c:833: warning: Excess function parameter 'work' description in 'queue_delayed_kthread_work'
kernel/sys.c:1: warning: no structured comments found
Great catch. Please, find below a fixed version of the patch.
From d20fa44b6a62a33098e4465c1bb0baf522b177f3 Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek@suse.com>
Date: Wed, 9 Dec 2015 15:53:03 +0100
Subject: [PATCH 7/7] kthread: Initial support for delayed kthread work
We are going to use kthread_worker more widely and delayed works
will be pretty useful.
The implementation is inspired by workqueues. It uses a timer to
queue the work after the requested delay. If the delay is zero,
the work is queued immediately.
In compare with workqueues, each work is associated with a single
worker (kthread). Therefore the implementation could be much easier.
In particular, we use the worker->lock to synchronize all the
operations with the work. We do not need any atomic operation
with a flags variable.
In fact, we do not need any state variable at all. Instead, we
add a list of delayed works into the worker. Then the pending
work is listed either in the list of queued or delayed works.
And the existing check of pending works is the same even for
the delayed ones.
A work must not be assigned to another worker unless reinitialized.
Therefore the timer handler might expect that dwork->work->worker
is valid and it could simply take the lock. We just add some
sanity checks to help with debugging a potential misuse.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 33 ++++++++++++++++
kernel/kthread.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 135 insertions(+)
@@ -747,6 +748,107 @@ bool queue_kthread_work(struct kthread_worker *worker,}EXPORT_SYMBOL_GPL(queue_kthread_work);+/**+*delayed_kthread_work_timer_fn-callbackthatqueuestheassociateddelayed+*kthreadworkwhenthetimerexpires.+*@__data:pointertothedataassociatedwiththetimer+*+*Theformatofthefunctionisdefinedbystructtimer_list.+*Itshouldhavebeencalledfromirqsafetimerwithirqalreadyoff.+*/+voiddelayed_kthread_work_timer_fn(unsignedlong__data)+{+structdelayed_kthread_work*dwork=+(structdelayed_kthread_work*)__data;+structkthread_work*work=&dwork->work;+structkthread_worker*worker=work->worker;++/*+*Thismighthappenwhenapendingworkisreinitialized.+*Itmeansthatitisusedawrongway.+*/+if(WARN_ON_ONCE(!worker))+return;++spin_lock(&worker->lock);+/* Work must not be used with more workers, see queue_kthread_work(). */+WARN_ON_ONCE(work->worker!=worker);++/* Move the work from worker->delayed_work_list. */+WARN_ON_ONCE(list_empty(&work->node));+list_del_init(&work->node);+insert_kthread_work(worker,work,&worker->work_list);++spin_unlock(&worker->lock);+}+EXPORT_SYMBOL(delayed_kthread_work_timer_fn);++void__queue_delayed_kthread_work(structkthread_worker*worker,+structdelayed_kthread_work*dwork,+unsignedlongdelay)+{+structtimer_list*timer=&dwork->timer;+structkthread_work*work=&dwork->work;++WARN_ON_ONCE(timer->function!=delayed_kthread_work_timer_fn||+timer->data!=(unsignedlong)dwork);++/*+*If@delayis0,queue@dwork->workimmediately.Thisisfor+*bothoptimizationandcorrectness.Theearliest@timercan+*expireisontheclosestnexttickanddelayed_workusersdepend+*onthatthere'snosuchdelaywhen@delayis0.+*/+if(!delay){+insert_kthread_work(worker,work,&worker->work_list);+return;+}++/* Be paranoid and try to detect possible races already now. */+insert_kthread_work_sanity_check(worker,work);++list_add(&work->node,&worker->delayed_work_list);+work->worker=worker;+timer_stats_timer_set_start_info(&dwork->timer);+timer->expires=jiffies+delay;+add_timer(timer);+}++/**+*queue_delayed_kthread_work-queuetheassociatedkthreadwork+*afteradelay.+*@worker:targetkthread_worker+*@dwork:delayed_kthread_worktoqueue+*@delay:numberofjiffiestowaitbeforequeuing+*+*Iftheworkhasnotbeenpendingitstartsatimerthatwillqueue+*theworkafterthegiven@delay.If@delayiszero,itqueuesthe+*workimmediately.+*+*Return:%falseifthe@workhasalreadybeenpending.Itmeansthat+*eitherthetimerwasrunningortheworkwasqueued.Itreturns%true+*otherwise.+*/+boolqueue_delayed_kthread_work(structkthread_worker*worker,+structdelayed_kthread_work*dwork,+unsignedlongdelay)+{+structkthread_work*work=&dwork->work;+unsignedlongflags;+boolret=false;++spin_lock_irqsave(&worker->lock,flags);++if(list_empty(&work->node)){+__queue_delayed_kthread_work(worker,dwork,delay);+ret=true;+}++spin_unlock_irqrestore(&worker->lock,flags);+returnret;+}+EXPORT_SYMBOL_GPL(queue_delayed_kthread_work);+structkthread_flush_work{structkthread_workwork;structcompletiondone;
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:58:17
We are going to use kthread workers more widely and sometimes we will need
to make sure that the work is neither pending nor running.
This patch implements cancel_*_sync() operations as inspired by
workqueues. Well, we are synchronized against the other operations
via the worker lock, we use del_timer_sync() and a counter to count
parallel cancel operations. Therefore the implementation might be easier.
First, we check if a worker is assigned. If not, the work has newer
been queued after it was initialized.
Second, we take the worker lock. It must be the right one. The work must
not be assigned to another worker unless it is initialized in between.
Third, we try to cancel the timer when it exists. The timer is deleted
synchronously to make sure that the timer call back is not running.
We hold the worker->lock at this point. To avoid a possible deadlock,
the timer callback need to check work->canceling counter and give up
when it has a non-zero value.
Fourth, we try to remove the work from a worker list. It might be
the list of normal or delayed works. Note that the work is in
the delayed list when the timer callback raced with del_timer_sync()
and was unable to move the work.
Fifth, if the work is running, we call flush_kthread_work(). It might
take an arbitrary time. In the meantime, queuing of the work is blocked
by the new canceling counter.
As already mentioned, the check for a pending kthread work is done under
a lock. In compare with workqueues, we do not need to fight for a single
PENDING bit to block other operations. Therefore do not suffer from
the thundering storm problem and all parallel canceling jobs might use
kthread_work_flush(). Any queuing is blocked until the counter is zero.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 5 ++
kernel/kthread.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 148 insertions(+), 3 deletions(-)
@@ -77,6 +77,8 @@ struct kthread_work {structlist_headnode;kthread_work_func_tfunc;structkthread_worker*worker;+/* Number of canceling calls that are running at the moment. */+intcanceling;};structdelayed_kthread_work{
@@ -768,7 +780,22 @@ void delayed_kthread_work_timer_fn(unsigned long __data)if(WARN_ON_ONCE(!worker))return;-spin_lock(&worker->lock);+/*+*Wemightbeunabletotakethelockifsomeoneistryingto+*cancelthisworkandcallsdel_timer_sync()whenthiscallback+*hasalreadybeenremovedfromthetimerlist.+*/+while(!spin_trylock(&worker->lock)){+/*+*Busywaitwithspin_is_locked()toavoidcachebouncing.+*Breakwhencancelingissettoavoidadeadlock.+*/+do{+if(work->canceling)+return;+cpu_relax();+}while(spin_is_locked(&worker->lock));+}/* Work must not be used with more workers, see queue_kthread_work(). */WARN_ON_ONCE(work->worker!=worker);
@@ -896,6 +923,119 @@ void flush_kthread_work(struct kthread_work *work)}EXPORT_SYMBOL_GPL(flush_kthread_work);+/*+*Thisfunctionremovestheworkfromtheworkerqueue.Alsoitmakessure+*thatitwon'tgetqueuedlaterviathedelayedwork'stimer.+*+*Theworkmightstillbeinusewhenthisfunctionfinishes.Seethe+*current_workproceedbytheworker.+*+*Return:%trueif@workwaspendingandsuccessfullycanceled,+*%falseif@workwasnotpending+*/+staticbool__cancel_kthread_work(structkthread_work*work,boolis_dwork)+{+/* Try to cancel the timer if exists. */+if(is_dwork){+structdelayed_kthread_work*dwork=+container_of(work,structdelayed_kthread_work,work);++del_timer_sync(&dwork->timer);+}++/*+*Trytoremovetheworkfromaworkerlist.Itmighteither+*befromworker->work_listorfromworker->delayed_work_list.+*+*Notethattheworkisstillinthedelayedlistwhendel_timer_sync()+*racedwiththetimercallback.Inthiscasethecallbackwasnotable+*totakethelockandmovetheworktothenormallist.+*/+if(!list_empty(&work->node)){+list_del_init(&work->node);+returntrue;+}++returnfalse;+}++staticbool__cancel_kthread_work_sync(structkthread_work*work,boolis_dwork)+{+structkthread_worker*worker=work->worker;+unsignedlongflags;+intret=false;++if(!worker)+gotoout;++spin_lock_irqsave(&worker->lock,flags);+/* Work must not be used with more workers, see queue_kthread_work(). */+WARN_ON_ONCE(worker!=work->worker);++/*+*work->cancelinghastwofunctionshere.Itblocksqueueinguntil+*thecanceloperationiscomplete.Alsoittellsthetimercallback+*thatitcannottaketheworkerlock.Itpreventsadeadlockbetween+*thecallbackanddel_timer_sync().+*/+work->canceling++;+ret=__cancel_kthread_work(work,is_dwork);++if(worker->current_work!=work)+gotoout_fast;++spin_unlock_irqrestore(&worker->lock,flags);+flush_kthread_work(work);+/*+*Nobodyisallowedtoswitchtheworkerorqueuethework+*when.cancelingisset.+*/+spin_lock_irqsave(&worker->lock,flags);++out_fast:+work->canceling--;+spin_unlock_irqrestore(&worker->lock,flags);+out:+returnret;+}++/**+*cancel_kthread_work_sync-cancelakthreadworkandwaitforittofinish+*@work:thekthreadworktocancel+*+*Cancel@workandwaitforitsexecutiontofinish.Thisfunction+*canbeusedeveniftheworkre-queuesitself.Onreturnfromthis+*function,@workisguaranteedtobenotpendingorexecutingonanyCPU.+*+*cancel_kthread_work_sync(&delayed_work->work)mustnotbeusedfor+*delayed_work's.Usecancel_delayed_kthread_work_sync()instead.++*Thecallermustensurethattheworkeronwhich@workwaslast+*queuedcan'tbedestroyedbeforethisfunctionreturns.+*+*Return:%trueif@workwaspending,%falseotherwise.+*/+boolcancel_kthread_work_sync(structkthread_work*work)+{+return__cancel_kthread_work_sync(work,false);+}+EXPORT_SYMBOL_GPL(cancel_kthread_work_sync);++/**+*cancel_delayed_kthread_work_sync-canceladelayedkthreadworkand+*waitforittofinish.+*@dwork:thedelayedkthreadworktocancel+*+*Thisiscancel_kthread_work_sync()fordelayedworks.+*+*Return:%trueif@dworkwaspending,%falseotherwise.+*/+boolcancel_delayed_kthread_work_sync(structdelayed_kthread_work*dwork)+{+return__cancel_kthread_work_sync(&dwork->work,true);+}+EXPORT_SYMBOL_GPL(cancel_delayed_kthread_work_sync);+/***flush_kthread_worker-flushallcurrentworksonakthread_worker*@worker:workertoflush
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: kbuild test robot <hidden> Date: 2016-02-22 16:51:08
Hi Petr,
[auto build test WARNING on soc-thermal/next]
[also build test WARNING on v4.5-rc5 next-20160222]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Petr-Mladek/kthread-Use-kthread-worker-API-more-widely/20160222-230250
base: https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal next
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
include/linux/init.h:1: warning: no structured comments found
kernel/kthread.c:860: warning: No description found for parameter 'dwork'
kernel/kthread.c:860: warning: No description found for parameter 'delay'
kernel/kthread.c:860: warning: Excess function parameter 'work' description in 'queue_delayed_kthread_work'
quoted
kernel/kthread.c:1012: warning: bad line:
kernel/sys.c:1: warning: no structured comments found
drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
drivers/dma-buf/reservation.c:1: warning: no structured comments found
include/linux/reservation.h:1: warning: no structured comments found
include/linux/spi/spi.h:540: warning: No description found for parameter 'max_transfer_size'
vim +1012 kernel/kthread.c
854 * either the timer was running or the work was queued. It returns %true
855 * otherwise.
856 */
857 bool queue_delayed_kthread_work(struct kthread_worker *worker,
858 struct delayed_kthread_work *dwork,
859 unsigned long delay)
> 860 {
861 struct kthread_work *work = &dwork->work;
862 unsigned long flags;
863 bool ret = false;
864
865 spin_lock_irqsave(&worker->lock, flags);
866
867 if (!queuing_blocked(work)) {
868 __queue_delayed_kthread_work(worker, dwork, delay);
869 ret = true;
870 }
871
872 spin_unlock_irqrestore(&worker->lock, flags);
873 return ret;
874 }
875 EXPORT_SYMBOL_GPL(queue_delayed_kthread_work);
876
877 struct kthread_flush_work {
878 struct kthread_work work;
879 struct completion done;
880 };
881
882 static void kthread_flush_work_fn(struct kthread_work *work)
883 {
884 struct kthread_flush_work *fwork =
885 container_of(work, struct kthread_flush_work, work);
886 complete(&fwork->done);
887 }
888
889 /**
890 * flush_kthread_work - flush a kthread_work
891 * @work: work to flush
892 *
893 * If @work is queued or executing, wait for it to finish execution.
894 */
895 void flush_kthread_work(struct kthread_work *work)
896 {
897 struct kthread_flush_work fwork = {
898 KTHREAD_WORK_INIT(fwork.work, kthread_flush_work_fn),
899 COMPLETION_INITIALIZER_ONSTACK(fwork.done),
900 };
901 struct kthread_worker *worker;
902 bool noop = false;
903
904 worker = work->worker;
905 if (!worker)
906 return;
907
908 spin_lock_irq(&worker->lock);
909 /* Work must not be used with more workers, see queue_kthread_work(). */
910 WARN_ON_ONCE(work->worker != worker);
911
912 if (!list_empty(&work->node))
913 insert_kthread_work(worker, &fwork.work, work->node.next);
914 else if (worker->current_work == work)
915 insert_kthread_work(worker, &fwork.work, worker->work_list.next);
916 else
917 noop = true;
918
919 spin_unlock_irq(&worker->lock);
920
921 if (!noop)
922 wait_for_completion(&fwork.done);
923 }
924 EXPORT_SYMBOL_GPL(flush_kthread_work);
925
926 /*
927 * This function removes the work from the worker queue. Also it makes sure
928 * that it won't get queued later via the delayed work's timer.
929 *
930 * The work might still be in use when this function finishes. See the
931 * current_work proceed by the worker.
932 *
933 * Return: %true if @work was pending and successfully canceled,
934 * %false if @work was not pending
935 */
936 static bool __cancel_kthread_work(struct kthread_work *work, bool is_dwork)
937 {
938 /* Try to cancel the timer if exists. */
939 if (is_dwork) {
940 struct delayed_kthread_work *dwork =
941 container_of(work, struct delayed_kthread_work, work);
942
943 del_timer_sync(&dwork->timer);
944 }
945
946 /*
947 * Try to remove the work from a worker list. It might either
948 * be from worker->work_list or from worker->delayed_work_list.
949 *
950 * Note that the work is still in the delayed list when del_timer_sync()
951 * raced with the timer callback. In this case the callback was not able
952 * to take the lock and move the work to the normal list.
953 */
954 if (!list_empty(&work->node)) {
955 list_del_init(&work->node);
956 return true;
957 }
958
959 return false;
960 }
961
962 static bool __cancel_kthread_work_sync(struct kthread_work *work, bool is_dwork)
963 {
964 struct kthread_worker *worker = work->worker;
965 unsigned long flags;
966 int ret = false;
967
968 if (!worker)
969 goto out;
970
971 spin_lock_irqsave(&worker->lock, flags);
972 /* Work must not be used with more workers, see queue_kthread_work(). */
973 WARN_ON_ONCE(worker != work->worker);
974
975 /*
976 * work->canceling has two functions here. It blocks queueing until
977 * the cancel operation is complete. Also it tells the timer callback
978 * that it cannot take the worker lock. It prevents a deadlock between
979 * the callback and del_timer_sync().
980 */
981 work->canceling++;
982 ret = __cancel_kthread_work(work, is_dwork);
983
984 if (worker->current_work != work)
985 goto out_fast;
986
987 spin_unlock_irqrestore(&worker->lock, flags);
988 flush_kthread_work(work);
989 /*
990 * Nobody is allowed to switch the worker or queue the work
991 * when .canceling is set.
992 */
993 spin_lock_irqsave(&worker->lock, flags);
994
995 out_fast:
996 work->canceling--;
997 spin_unlock_irqrestore(&worker->lock, flags);
998 out:
999 return ret;
1000 }
1001
1002 /**
1003 * cancel_kthread_work_sync - cancel a kthread work and wait for it to finish
1004 * @work: the kthread work to cancel
1005 *
1006 * Cancel @work and wait for its execution to finish. This function
1007 * can be used even if the work re-queues itself. On return from this
1008 * function, @work is guaranteed to be not pending or executing on any CPU.
1009 *
1010 * cancel_kthread_work_sync(&delayed_work->work) must not be used for
1011 * delayed_work's. Use cancel_delayed_kthread_work_sync() instead.
1012
1013 * The caller must ensure that the worker on which @work was last
1014 * queued can't be destroyed before this function returns.
1015 *
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-24 16:18:10
On Tue 2016-02-23 00:50:22, kbuild test robot wrote:
Hi Petr,
[auto build test WARNING on soc-thermal/next]
[also build test WARNING on v4.5-rc5 next-20160222]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Petr-Mladek/kthread-Use-kthread-worker-API-more-widely/20160222-230250
base: https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal next
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
include/linux/init.h:1: warning: no structured comments found
kernel/kthread.c:860: warning: No description found for parameter 'dwork'
kernel/kthread.c:860: warning: No description found for parameter 'delay'
kernel/kthread.c:860: warning: Excess function parameter 'work' description in 'queue_delayed_kthread_work'
quoted
quoted
kernel/kthread.c:1012: warning: bad line:
kernel/sys.c:1: warning: no structured comments found
drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
vim +1012 kernel/kthread.c
1001
1002 /**
1003 * cancel_kthread_work_sync - cancel a kthread work and wait for it to finish
1004 * @work: the kthread work to cancel
1005 *
1006 * Cancel @work and wait for its execution to finish. This function
1007 * can be used even if the work re-queues itself. On return from this
1008 * function, @work is guaranteed to be not pending or executing on any CPU.
1009 *
1010 * cancel_kthread_work_sync(&delayed_work->work) must not be used for
1011 * delayed_work's. Use cancel_delayed_kthread_work_sync() instead.
quoted
1012
1013 * The caller must ensure that the worker on which @work was last
1014 * queued can't be destroyed before this function returns.
1015 *
Ups, there was missing an asterisk. Please, find the fixed patch
below.
From 58d0437b73fb368770e62ea1893e3253dc8e5c7d Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek@suse.com>
Date: Fri, 19 Feb 2016 15:19:34 +0100
Subject: [PATCH 8/8] kthread: Allow to cancel kthread work
We are going to use kthread workers more widely and sometimes we will need
to make sure that the work is neither pending nor running.
This patch implements cancel_*_sync() operations as inspired by
workqueues. Well, we are synchronized against the other operations
via the worker lock, we use del_timer_sync() and a counter to count
parallel cancel operations. Therefore the implementation might be easier.
First, we check if a worker is assigned. If not, the work has newer
been queued after it was initialized.
Second, we take the worker lock. It must be the right one. The work must
not be assigned to another worker unless it is initialized in between.
Third, we try to cancel the timer when it exists. The timer is deleted
synchronously to make sure that the timer call back is not running.
We hold the worker->lock at this point. To avoid a possible deadlock,
the timer callback need to check work->canceling counter and give up
when it has a non-zero value.
Fourth, we try to remove the work from a worker list. It might be
the list of normal or delayed works. Note that the work is in
the delayed list when the timer callback raced with del_timer_sync()
and was unable to move the work.
Fifth, if the work is running, we call flush_kthread_work(). It might
take an arbitrary time. In the meantime, queuing of the work is blocked
by the new canceling counter.
As already mentioned, the check for a pending kthread work is done under
a lock. In compare with workqueues, we do not need to fight for a single
PENDING bit to block other operations. Therefore do not suffer from
the thundering storm problem and all parallel canceling jobs might use
kthread_work_flush(). Any queuing is blocked until the counter is zero.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 5 ++
kernel/kthread.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 148 insertions(+), 3 deletions(-)
@@ -77,6 +77,8 @@ struct kthread_work {structlist_headnode;kthread_work_func_tfunc;structkthread_worker*worker;+/* Number of canceling calls that are running at the moment. */+intcanceling;};structdelayed_kthread_work{
@@ -770,7 +782,22 @@ void delayed_kthread_work_timer_fn(unsigned long __data)if(WARN_ON_ONCE(!worker))return;-spin_lock(&worker->lock);+/*+*Wemightbeunabletotakethelockifsomeoneistryingto+*cancelthisworkandcallsdel_timer_sync()whenthiscallback+*hasalreadybeenremovedfromthetimerlist.+*/+while(!spin_trylock(&worker->lock)){+/*+*Busywaitwithspin_is_locked()toavoidcachebouncing.+*Breakwhencancelingissettoavoidadeadlock.+*/+do{+if(work->canceling)+return;+cpu_relax();+}while(spin_is_locked(&worker->lock));+}/* Work must not be used with more workers, see queue_kthread_work(). */WARN_ON_ONCE(work->worker!=worker);
@@ -898,6 +925,119 @@ void flush_kthread_work(struct kthread_work *work)}EXPORT_SYMBOL_GPL(flush_kthread_work);+/*+*Thisfunctionremovestheworkfromtheworkerqueue.Alsoitmakessure+*thatitwon'tgetqueuedlaterviathedelayedwork'stimer.+*+*Theworkmightstillbeinusewhenthisfunctionfinishes.Seethe+*current_workproceedbytheworker.+*+*Return:%trueif@workwaspendingandsuccessfullycanceled,+*%falseif@workwasnotpending+*/+staticbool__cancel_kthread_work(structkthread_work*work,boolis_dwork)+{+/* Try to cancel the timer if exists. */+if(is_dwork){+structdelayed_kthread_work*dwork=+container_of(work,structdelayed_kthread_work,work);++del_timer_sync(&dwork->timer);+}++/*+*Trytoremovetheworkfromaworkerlist.Itmighteither+*befromworker->work_listorfromworker->delayed_work_list.+*+*Notethattheworkisstillinthedelayedlistwhendel_timer_sync()+*racedwiththetimercallback.Inthiscasethecallbackwasnotable+*totakethelockandmovetheworktothenormallist.+*/+if(!list_empty(&work->node)){+list_del_init(&work->node);+returntrue;+}++returnfalse;+}++staticbool__cancel_kthread_work_sync(structkthread_work*work,boolis_dwork)+{+structkthread_worker*worker=work->worker;+unsignedlongflags;+intret=false;++if(!worker)+gotoout;++spin_lock_irqsave(&worker->lock,flags);+/* Work must not be used with more workers, see queue_kthread_work(). */+WARN_ON_ONCE(worker!=work->worker);++/*+*work->cancelinghastwofunctionshere.Itblocksqueueinguntil+*thecanceloperationiscomplete.Alsoittellsthetimercallback+*thatitcannottaketheworkerlock.Itpreventsadeadlockbetween+*thecallbackanddel_timer_sync().+*/+work->canceling++;+ret=__cancel_kthread_work(work,is_dwork);++if(worker->current_work!=work)+gotoout_fast;++spin_unlock_irqrestore(&worker->lock,flags);+flush_kthread_work(work);+/*+*Nobodyisallowedtoswitchtheworkerorqueuethework+*when.cancelingisset.+*/+spin_lock_irqsave(&worker->lock,flags);++out_fast:+work->canceling--;+spin_unlock_irqrestore(&worker->lock,flags);+out:+returnret;+}++/**+*cancel_kthread_work_sync-cancelakthreadworkandwaitforittofinish+*@work:thekthreadworktocancel+*+*Cancel@workandwaitforitsexecutiontofinish.Thisfunction+*canbeusedeveniftheworkre-queuesitself.Onreturnfromthis+*function,@workisguaranteedtobenotpendingorexecutingonanyCPU.+*+*cancel_kthread_work_sync(&delayed_work->work)mustnotbeusedfor+*delayed_work's.Usecancel_delayed_kthread_work_sync()instead.+*+*Thecallermustensurethattheworkeronwhich@workwaslast+*queuedcan'tbedestroyedbeforethisfunctionreturns.+*+*Return:%trueif@workwaspending,%falseotherwise.+*/+boolcancel_kthread_work_sync(structkthread_work*work)+{+return__cancel_kthread_work_sync(work,false);+}+EXPORT_SYMBOL_GPL(cancel_kthread_work_sync);++/**+*cancel_delayed_kthread_work_sync-canceladelayedkthreadworkand+*waitforittofinish.+*@dwork:thedelayedkthreadworktocancel+*+*Thisiscancel_kthread_work_sync()fordelayedworks.+*+*Return:%trueif@dworkwaspending,%falseotherwise.+*/+boolcancel_delayed_kthread_work_sync(structdelayed_kthread_work*dwork)+{+return__cancel_kthread_work_sync(&dwork->work,true);+}+EXPORT_SYMBOL_GPL(cancel_delayed_kthread_work_sync);+/***flush_kthread_worker-flushallcurrentworksonakthread_worker*@worker:workertoflush
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Peter Zijlstra <peterz@infradead.org> Date: 2016-02-25 12:59:47
On Wed, Feb 24, 2016 at 05:18:05PM +0100, Petr Mladek wrote:
quoted hunk
@@ -770,7 +782,22 @@ void delayed_kthread_work_timer_fn(unsigned long __data) if (WARN_ON_ONCE(!worker)) return;- spin_lock(&worker->lock);+ /*+ * We might be unable to take the lock if someone is trying to+ * cancel this work and calls del_timer_sync() when this callback+ * has already been removed from the timer list.+ */+ while (!spin_trylock(&worker->lock)) {+ /*+ * Busy wait with spin_is_locked() to avoid cache bouncing.+ * Break when canceling is set to avoid a deadlock.+ */+ do {+ if (work->canceling)+ return;+ cpu_relax();+ } while (spin_is_locked(&worker->lock));+ } /* Work must not be used with more workers, see queue_kthread_work(). */ WARN_ON_ONCE(work->worker != worker);
This is pretty vile; why can't you drop the lock over del_timer_sync() ?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-26 15:38:23
On Thu 2016-02-25 13:59:32, Peter Zijlstra wrote:
On Wed, Feb 24, 2016 at 05:18:05PM +0100, Petr Mladek wrote:
quoted
@@ -770,7 +782,22 @@ void delayed_kthread_work_timer_fn(unsigned long __data) if (WARN_ON_ONCE(!worker)) return;- spin_lock(&worker->lock);+ /*+ * We might be unable to take the lock if someone is trying to+ * cancel this work and calls del_timer_sync() when this callback+ * has already been removed from the timer list.+ */+ while (!spin_trylock(&worker->lock)) {+ /*+ * Busy wait with spin_is_locked() to avoid cache bouncing.+ * Break when canceling is set to avoid a deadlock.+ */+ do {+ if (work->canceling)+ return;+ cpu_relax();+ } while (spin_is_locked(&worker->lock));+ } /* Work must not be used with more workers, see queue_kthread_work(). */ WARN_ON_ONCE(work->worker != worker);
This is pretty vile; why can't you drop the lock over del_timer_sync() ?
We would need to take the lock later and check if nobody has set the timer
again in the meantime.
Now, timer_active() check is not reliable. It does not check if the
timer handler is running at the moment. I tried to implement
a safe timer_active()[1] but nobody was keen to take it.
Even if we have that timer_active() check, we would need to add
some relock/check/try_again stuff around the del_timer_sync().
So, it would just move the complexity somewhere else.
I think that the current solution is quite elegant after all.
Thanks a lot Tejun for the idea.
Reference:
[1] http://thread.gmane.org/gmane.linux.kernel.mm/144964/focus=144965
Thanks a lot for review,
Petr
From: Peter Zijlstra <peterz@infradead.org> Date: 2016-02-26 16:26:10
On Fri, Feb 26, 2016 at 04:38:18PM +0100, Petr Mladek wrote:
On Thu 2016-02-25 13:59:32, Peter Zijlstra wrote:
quoted
On Wed, Feb 24, 2016 at 05:18:05PM +0100, Petr Mladek wrote:
quoted
@@ -770,7 +782,22 @@ void delayed_kthread_work_timer_fn(unsigned long __data) if (WARN_ON_ONCE(!worker)) return;- spin_lock(&worker->lock);+ /*+ * We might be unable to take the lock if someone is trying to+ * cancel this work and calls del_timer_sync() when this callback+ * has already been removed from the timer list.+ */+ while (!spin_trylock(&worker->lock)) {+ /*+ * Busy wait with spin_is_locked() to avoid cache bouncing.+ * Break when canceling is set to avoid a deadlock.+ */+ do {+ if (work->canceling)+ return;+ cpu_relax();+ } while (spin_is_locked(&worker->lock));+ } /* Work must not be used with more workers, see queue_kthread_work(). */ WARN_ON_ONCE(work->worker != worker);
This is pretty vile; why can't you drop the lock over del_timer_sync() ?
We would need to take the lock later and check if nobody has set the timer
again in the meantime.
Well, if ->cancelling is !0, nobody should be re-queueing, re-arming
timers etc.., right?
And since you do add_timer() while holding the spinlock, this should all
work out, no?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-26 17:01:02
On Fri 2016-02-26 17:25:52, Peter Zijlstra wrote:
On Fri, Feb 26, 2016 at 04:38:18PM +0100, Petr Mladek wrote:
quoted
On Thu 2016-02-25 13:59:32, Peter Zijlstra wrote:
quoted
On Wed, Feb 24, 2016 at 05:18:05PM +0100, Petr Mladek wrote:
quoted
@@ -770,7 +782,22 @@ void delayed_kthread_work_timer_fn(unsigned long __data) if (WARN_ON_ONCE(!worker)) return;- spin_lock(&worker->lock);+ /*+ * We might be unable to take the lock if someone is trying to+ * cancel this work and calls del_timer_sync() when this callback+ * has already been removed from the timer list.+ */+ while (!spin_trylock(&worker->lock)) {+ /*+ * Busy wait with spin_is_locked() to avoid cache bouncing.+ * Break when canceling is set to avoid a deadlock.+ */+ do {+ if (work->canceling)+ return;+ cpu_relax();+ } while (spin_is_locked(&worker->lock));+ } /* Work must not be used with more workers, see queue_kthread_work(). */ WARN_ON_ONCE(work->worker != worker);
This is pretty vile; why can't you drop the lock over del_timer_sync() ?
We would need to take the lock later and check if nobody has set the timer
again in the meantime.
Well, if ->cancelling is !0, nobody should be re-queueing, re-arming
timers etc.., right?
Which rings a bell. __cancel_kthread_work()/del_timer_sync() is used
also from mod_timer_sync() and we do not increment ->canceling there.
See the 9th patch. It is racy and I have to fix it.
And since you do add_timer() while holding the spinlock, this should all
work out, no?
Interesting idea. Yes, it should work. But is this really easier? The
try_again/relock/recheck code is not trivial either.
I personally slightly more prefer the current code. I am open
to use your version ff you persist on it. But I also do not want
to end up in a ping-pong re-implementation as Tejun king of suggested
the current code.
Thanks a lot,
Petr
From: Peter Zijlstra <peterz@infradead.org> Date: 2016-02-27 15:16:46
On Fri, Feb 26, 2016 at 06:00:56PM +0100, Petr Mladek wrote:
On Fri 2016-02-26 17:25:52, Peter Zijlstra wrote:
quoted
On Fri, Feb 26, 2016 at 04:38:18PM +0100, Petr Mladek wrote:
quoted
On Thu 2016-02-25 13:59:32, Peter Zijlstra wrote:
quoted
On Wed, Feb 24, 2016 at 05:18:05PM +0100, Petr Mladek wrote:
quoted
@@ -770,7 +782,22 @@ void delayed_kthread_work_timer_fn(unsigned long __data) if (WARN_ON_ONCE(!worker)) return;- spin_lock(&worker->lock);+ /*+ * We might be unable to take the lock if someone is trying to+ * cancel this work and calls del_timer_sync() when this callback+ * has already been removed from the timer list.+ */+ while (!spin_trylock(&worker->lock)) {+ /*+ * Busy wait with spin_is_locked() to avoid cache bouncing.+ * Break when canceling is set to avoid a deadlock.+ */+ do {+ if (work->canceling)+ return;+ cpu_relax();+ } while (spin_is_locked(&worker->lock));+ } /* Work must not be used with more workers, see queue_kthread_work(). */ WARN_ON_ONCE(work->worker != worker);
quoted
And since you do add_timer() while holding the spinlock, this should all
work out, no?
Interesting idea. Yes, it should work. But is this really easier? The
try_again/relock/recheck code is not trivial either.
The trylock loop has very bad worst case behaviour, it really is best to
avoid such patterns if at all possible.
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:58:26
There are situations when we need to modify the delay of a delayed kthread
work. For example, when the work depends on an event and the initial delay
means a timeout. Then we want to queue the work immediately when the event
happens.
This patch implements mod_delayed_kthread_work() as inspired workqueues.
It cancels the timer, removes the work from any worker list and queues it
again with the given timeout.
A very special case is when the work is being canceled at the same time.
cancel_*kthread_work_sync() operation blocks queuing until the running
work finishes. Therefore we do nothing and let cancel() win. This should
not normally happen as the caller is supposed to synchronize these
operations a reasonable way.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 4 ++++
kernel/kthread.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+)
@@ -959,6 +959,51 @@ static bool __cancel_kthread_work(struct kthread_work *work, bool is_dwork)returnfalse;}+/**+*mod_delayed_kthread_work-modifydelayoforqueueadelayedkthreadwork+*@worker:kthreadworkertouse+*@dwork:delayedkthreadworktoqueue+*@delay:numberofjiffiestowaitbeforequeuing+*+*If@dworkisidle,equivalenttoqueue_delayed_kthreadwork().Otherwise,+*modify@dwork'stimersothatitexpiresafter@delay.If@delayiszero,+*@workisguaranteedtobequeuedimmediately.+*+*Return:%trueif@dworkwaspendinganditstimerwasmodified,+*%falseotherwise.+*+*Aspecialcaseiswhencancel_work_sync()isrunninginparallel.+*Itblocksfurtherqueuing.Weletthecancel()winandreturn%false.+*Thecallerissupposedtosynchronizetheseoperationsareasonableway.+*+*ThisfunctionissafetocallfromanycontextincludingIRQhandler.+*See__cancel_kthread_work()anddelayed_kthread_work_timer_fn()+*fordetails.+*/+boolmod_delayed_kthread_work(structkthread_worker*worker,+structdelayed_kthread_work*dwork,+unsignedlongdelay)+{+structkthread_work*work=&dwork->work;+unsignedlongflags;+intret=false;++spin_lock_irqsave(&worker->lock,flags);+/* Work must not be used with more workers, see queue_kthread_work() */+WARN_ON_ONCE(work->worker&&work->worker!=worker);++if(work->canceling)+gotoout;++ret=__cancel_kthread_work(work,true);+__queue_delayed_kthread_work(worker,dwork,delay);++out:+spin_unlock_irqrestore(&worker->lock,flags);+returnret;+}+EXPORT_SYMBOL_GPL(mod_delayed_kthread_work);+staticbool__cancel_kthread_work_sync(structkthread_work*work,boolis_dwork){structkthread_worker*worker=work->worker;
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:58:37
This patch allows to make kthread worker freezable via a new @flags
parameter. It will allow to avoid an init work in some kthreads.
It currently does not affect the function of kthread_worker_fn()
but it might help to do some optimization or fixes eventually.
I currently do not know about any other use for the @flags
parameter but I believe that we will want more flags
in the future.
Finally, I hope that it will not cause confusion with @flags member
in struct kthread. Well, I guess that we will want to rework the
basic kthreads implementation once all kthreads are converted into
kthread workers or workqueues. It is possible that we will merge
the two structures.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 11 ++++++++---
kernel/kthread.c | 18 ++++++++++++------
2 files changed, 20 insertions(+), 9 deletions(-)
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
kernel/kthread.c:671: warning: No description found for parameter 'flags'
kernel/kthread.c:700: warning: No description found for parameter 'flags'
kernel/kthread.c:866: warning: No description found for parameter 'dwork'
kernel/kthread.c:866: warning: No description found for parameter 'delay'
kernel/kthread.c:866: warning: Excess function parameter 'work' description in 'queue_delayed_kthread_work'
kernel/kthread.c:1063: warning: bad line:
kernel/sys.c:1: warning: no structured comments found
drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
drivers/dma-buf/reservation.c:1: warning: no structured comments found
include/linux/reservation.h:1: warning: no structured comments found
include/linux/spi/spi.h:540: warning: No description found for parameter 'max_transfer_size'
vim +/flags +671 kernel/kthread.c
bf63ca4b Petr Mladek 2016-02-22 655
bf63ca4b Petr Mladek 2016-02-22 656 fail_task:
bf63ca4b Petr Mladek 2016-02-22 657 kfree(worker);
bf63ca4b Petr Mladek 2016-02-22 658 return ERR_CAST(task);
bf63ca4b Petr Mladek 2016-02-22 659 }
bf63ca4b Petr Mladek 2016-02-22 660
bf63ca4b Petr Mladek 2016-02-22 661 /**
bf63ca4b Petr Mladek 2016-02-22 662 * create_kthread_worker - create a kthread worker
bf63ca4b Petr Mladek 2016-02-22 663 * @namefmt: printf-style name for the kthread worker (task).
bf63ca4b Petr Mladek 2016-02-22 664 *
bf63ca4b Petr Mladek 2016-02-22 665 * Returns a pointer to the allocated worker on success, ERR_PTR(-ENOMEM)
bf63ca4b Petr Mladek 2016-02-22 666 * when the needed structures could not get allocated, and ERR_PTR(-EINTR)
bf63ca4b Petr Mladek 2016-02-22 667 * when the worker was SIGKILLed.
bf63ca4b Petr Mladek 2016-02-22 668 */
bf63ca4b Petr Mladek 2016-02-22 669 struct kthread_worker *
28590586 Petr Mladek 2016-02-22 670 create_kthread_worker(unsigned int flags, const char namefmt[], ...)
bf63ca4b Petr Mladek 2016-02-22 @671 {
bf63ca4b Petr Mladek 2016-02-22 672 struct kthread_worker *worker;
bf63ca4b Petr Mladek 2016-02-22 673 va_list args;
bf63ca4b Petr Mladek 2016-02-22 674
bf63ca4b Petr Mladek 2016-02-22 675 va_start(args, namefmt);
28590586 Petr Mladek 2016-02-22 676 worker = __create_kthread_worker(-1, flags, namefmt, args);
bf63ca4b Petr Mladek 2016-02-22 677 va_end(args);
bf63ca4b Petr Mladek 2016-02-22 678
bf63ca4b Petr Mladek 2016-02-22 679 return worker;
:::::: The code at line 671 was first introduced by commit
:::::: bf63ca4b5dec706d4b88418b06ccdc1a754b25e4 kthread: Add create_kthread_worker*()
:::::: TO: Petr Mladek [off-list ref]
:::::: CC: 0day robot [off-list ref]
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
kernel/kthread.c:671: warning: No description found for parameter 'flags'
kernel/kthread.c:700: warning: No description found for parameter 'flags'
Please find below an updated version of the patch that fixes the above
warnings:
From 66fbe9142d4761a7010a73eeb69ef315053684cb Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek@suse.com>
Date: Mon, 22 Jun 2015 15:05:16 +0200
Subject: [PATCH 10/10] kthread: Better support freezable kthread workers
This patch allows to make kthread worker freezable via a new @flags
parameter. It will allow to avoid an init work in some kthreads.
It currently does not affect the function of kthread_worker_fn()
but it might help to do some optimization or fixes eventually.
I currently do not know about any other use for the @flags
parameter but I believe that we will want more flags
in the future.
Finally, I hope that it will not cause confusion with @flags member
in struct kthread. Well, I guess that we will want to rework the
basic kthreads implementation once all kthreads are converted into
kthread workers or workqueues. It is possible that we will merge
the two structures.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 11 ++++++++---
kernel/kthread.c | 20 ++++++++++++++------
2 files changed, 22 insertions(+), 9 deletions(-)
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Heh, the flag was inspired by
WQ_FREEZABLE = 1 << 2, /* freeze during suspend */
from include/linux/workqueue.h. But it does not really matter.
I could change it to 1 << 0 if it makes people less curious.
Thanks a lot for review,
Petr
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:58:47
Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. In many cases it is unclear to say in which state
it is and sometimes it is done a wrong way.
The plan is to convert kthreads into kthread_worker or workqueues
API. It allows to split the functionality into separate operations.
It helps to make a better structure. Also it defines a clean state
where no locks are taken, IRQs blocked, the kthread might sleep
or even be safely migrated.
The kthread worker API is useful when we want to have a dedicated
single thread for the work. It helps to make sure that it is
available when needed. Also it allows a better control, e.g.
define a scheduling priority.
This patch converts khugepaged() in kthread worker API
because it modifies the scheduling.
It keeps the functionality except that we do not wakeup the worker
when it is already created and someone calls start() once again.
Note that kthread works get associated with a single kthread worker.
They must be initialized if we want to use them with another worker.
This is needed also when the worker is restarted.
set_freezable() is not needed because the kthread worker is
created as freezable.
set_user_nice() is called from start_stop_khugepaged(). It need
not be done from within the kthread.
The scan work must be queued only when the worker is available.
We have to use "khugepaged_mm_lock" to avoid a race between the check
and queuing. I admit that this was a bit easier before because wake_up()
was a nope when the kthread did not exist.
Also the scan work is queued only when the list of scanned pages is
not empty. It adds one check but it is cleaner.
They delay between scans is done using a delayed work.
Note that @khugepaged_wait waitqueue had two purposes. It was used
to wait between scans and when an allocation failed. It is still used
for the second purpose. Therefore it was renamed to better describe
the current use.
Also note that we could not longer check for kthread_should_stop()
in the works. The kthread used by the worker has to stay alive
until all queued works are finished. Instead, we use the existing
check khugepaged_enabled() that returns false when we are going down.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
mm/huge_memory.c | 138 ++++++++++++++++++++++++++++++-------------------------
1 file changed, 76 insertions(+), 62 deletions(-)
@@ -89,10 +89,16 @@ static unsigned int khugepaged_full_scans;staticunsignedintkhugepaged_scan_sleep_millisecs__read_mostly=10000;/* during fragmentation poll the hugepage allocator once every minute */staticunsignedintkhugepaged_alloc_sleep_millisecs__read_mostly=60000;-staticstructtask_struct*khugepaged_thread__read_mostly;++staticvoidkhugepaged_do_scan_func(structkthread_work*dummy);+staticvoidkhugepaged_cleanup_func(structkthread_work*dummy);+staticstructkthread_worker*khugepaged_worker;+staticstructdelayed_kthread_workkhugepaged_do_scan_work;+staticstructkthread_workkhugepaged_cleanup_work;+staticDEFINE_MUTEX(khugepaged_mutex);staticDEFINE_SPINLOCK(khugepaged_mm_lock);-staticDECLARE_WAIT_QUEUE_HEAD(khugepaged_wait);+staticDECLARE_WAIT_QUEUE_HEAD(khugepaged_alloc_wait);/**defaultcollapsehugepagesifthereisatleastoneptemappedlike*itwouldhavehappenedifthevmawaslargeenoughduringpage
@@ -177,29 +182,55 @@ static void set_recommended_min_free_kbytes(void)setup_per_zone_wmarks();}+staticintkhugepaged_has_work(void)+{+return!list_empty(&khugepaged_scan.mm_head);+}+staticintstart_stop_khugepaged(void){+structkthread_worker*worker;interr=0;+if(khugepaged_enabled()){-if(!khugepaged_thread)-khugepaged_thread=kthread_run(khugepaged,NULL,-"khugepaged");-if(IS_ERR(khugepaged_thread)){-pr_err("khugepaged: kthread_run(khugepaged) failed\n");-err=PTR_ERR(khugepaged_thread);-khugepaged_thread=NULL;-gotofail;+if(khugepaged_worker)+gotoout;++worker=create_kthread_worker(KTW_FREEZABLE,"khugepaged");+if(IS_ERR(worker)){+pr_err("khugepaged: failed to create kthread worker\n");+gotoout;}+set_user_nice(worker->task,MAX_NICE);-if(!list_empty(&khugepaged_scan.mm_head))-wake_up_interruptible(&khugepaged_wait);+/* Always initialize the works when the worker is started. */+init_delayed_kthread_work(&khugepaged_do_scan_work,+khugepaged_do_scan_func);+init_kthread_work(&khugepaged_cleanup_work,+khugepaged_cleanup_func);++/* Make the worker public and check for work synchronously. */+spin_lock(&khugepaged_mm_lock);+khugepaged_worker=worker;+if(khugepaged_has_work())+queue_delayed_kthread_work(worker,+&khugepaged_do_scan_work,+0);+spin_unlock(&khugepaged_mm_lock);set_recommended_min_free_kbytes();-}elseif(khugepaged_thread){-kthread_stop(khugepaged_thread);-khugepaged_thread=NULL;+}elseif(khugepaged_worker){+/* First, stop others from using the worker. */+spin_lock(&khugepaged_mm_lock);+worker=khugepaged_worker;+khugepaged_worker=NULL;+spin_unlock(&khugepaged_mm_lock);++cancel_delayed_kthread_work_sync(&khugepaged_do_scan_work);+queue_kthread_work(worker,&khugepaged_cleanup_work);+destroy_kthread_worker(worker);}-fail:+out:returnerr;}
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:58:58
Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. In many cases it is unclear to say in which state
it is and sometimes it is done a wrong way.
The plan is to convert kthreads into kthread_worker or workqueues
API. It allows to split the functionality into separate operations.
It helps to make a better structure. Also it defines a clean state
where no locks are taken, IRQs blocked, the kthread might sleep
or even be safely migrated.
The kthread worker API is useful when we want to have a dedicated
single thread for the work. It helps to make sure that it is
available when needed. Also it allows a better control, e.g.
define a scheduling priority.
This patch converts the ring buffer benchmark producer into a kthread
worker because it modifies the scheduling priority and policy.
Also, it is a benchmark. It makes CPU very busy. It will most likely
run only limited time. IMHO, it does not make sense to mess the system
workqueues with it.
The thread is split into two independent works. It might look more
complicated but it helped me to find a race in the sleeping part
that was fixed separately.
kthread_should_stop() could not longer be used inside the works
because it defines the life of the worker and it needs to stay
usable until all works are done. Instead, we add @test_end
global variable. It is set during normal termination in compare
with @test_error.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/trace/ring_buffer_benchmark.c | 133 ++++++++++++++++-------------------
1 file changed, 59 insertions(+), 74 deletions(-)
@@ -281,14 +289,14 @@ static void ring_buffer_producer(void)}while(ktime_before(end_time,timeout)&&!break_test());trace_printk("End ring buffer hammer\n");-if(consumer){+if(rb_consumer_worker){/* Init both completions here to avoid races */init_completion(&read_start);init_completion(&read_done);/* the completions must be visible before the finish var */smp_wmb();reader_finish=1;-wake_up_process(consumer);+wake_up_process(rb_consumer_worker->task);wait_for_completion(&read_done);}
@@ -366,68 +374,39 @@ static void ring_buffer_producer(void)}}-staticvoidwait_to_die(void)-{-set_current_state(TASK_INTERRUPTIBLE);-while(!kthread_should_stop()){-schedule();-set_current_state(TASK_INTERRUPTIBLE);-}-__set_current_state(TASK_RUNNING);-}--staticintring_buffer_consumer_thread(void*arg)+staticvoidrb_consumer_func(structkthread_work*dummy){-while(!break_test()){-complete(&read_start);--ring_buffer_consumer();+complete(&read_start);-set_current_state(TASK_INTERRUPTIBLE);-if(break_test())-break;-schedule();-}-__set_current_state(TASK_RUNNING);--if(!kthread_should_stop())-wait_to_die();--return0;+ring_buffer_consumer();}-staticintring_buffer_producer_thread(void*arg)+staticvoidrb_producer_hammer_func(structkthread_work*dummy){-while(!break_test()){-ring_buffer_reset(buffer);+if(break_test())+return;-if(consumer){-wake_up_process(consumer);-wait_for_completion(&read_start);-}--ring_buffer_producer();-if(break_test())-gotoout_kill;+ring_buffer_reset(buffer);-trace_printk("Sleeping for 10 secs\n");-set_current_state(TASK_INTERRUPTIBLE);-if(break_test())-gotoout_kill;-schedule_timeout(HZ*SLEEP_TIME);+if(rb_consumer_worker){+queue_kthread_work(rb_consumer_worker,&rb_consumer_work);+wait_for_completion(&read_start);}-out_kill:-__set_current_state(TASK_RUNNING);-if(!kthread_should_stop())-wait_to_die();+ring_buffer_producer();-return0;+if(break_test())+return;++trace_printk("Sleeping for 10 secs\n");+queue_delayed_kthread_work(rb_producer_worker,+&rb_producer_hammer_work,+HZ*SLEEP_TIME);}staticint__initring_buffer_benchmark_init(void){-intret;+intret=0;/* make a one meg buffer in overwite mode */buffer=ring_buffer_alloc(1000000,RB_FL_OVERWRITE);
@@ -435,19 +414,21 @@ static int __init ring_buffer_benchmark_init(void)return-ENOMEM;if(!disable_reader){-consumer=kthread_create(ring_buffer_consumer_thread,-NULL,"rb_consumer");-ret=PTR_ERR(consumer);-if(IS_ERR(consumer))+rb_consumer_worker=create_kthread_worker(0,"rb_consumer");+if(IS_ERR(rb_consumer_worker)){+ret=PTR_ERR(rb_consumer_worker);gotoout_fail;+}}-producer=kthread_run(ring_buffer_producer_thread,-NULL,"rb_producer");-ret=PTR_ERR(producer);--if(IS_ERR(producer))+rb_producer_worker=create_kthread_worker(0,"rb_producer");+if(IS_ERR(rb_producer_worker)){+ret=PTR_ERR(rb_producer_worker);gotoout_kill;+}++queue_delayed_kthread_work(rb_producer_worker,+&rb_producer_hammer_work,0);/**Runthemaslow-priobackgroundtasksbydefault:
@@ -457,24 +438,26 @@ static int __init ring_buffer_benchmark_init(void)structsched_paramparam={.sched_priority=consumer_fifo};-sched_setscheduler(consumer,SCHED_FIFO,¶m);+sched_setscheduler(rb_consumer_worker->task,+SCHED_FIFO,¶m);}else-set_user_nice(consumer,consumer_nice);+set_user_nice(rb_consumer_worker->task,consumer_nice);}if(producer_fifo>=0){structsched_paramparam={.sched_priority=producer_fifo};-sched_setscheduler(producer,SCHED_FIFO,¶m);+sched_setscheduler(rb_producer_worker->task,+SCHED_FIFO,¶m);}else-set_user_nice(producer,producer_nice);+set_user_nice(rb_producer_worker->task,producer_nice);return0;out_kill:-if(consumer)-kthread_stop(consumer);+if(rb_consumer_worker)+destroy_kthread_worker(rb_consumer_worker);out_fail:ring_buffer_free(buffer);
@@ -483,9 +466,11 @@ static int __init ring_buffer_benchmark_init(void)staticvoid__exitring_buffer_benchmark_exit(void){-kthread_stop(producer);-if(consumer)-kthread_stop(consumer);+test_end=1;+cancel_delayed_kthread_work_sync(&rb_producer_hammer_work);+destroy_kthread_worker(rb_producer_worker);+if(rb_consumer_worker)+destroy_kthread_worker(rb_consumer_worker);ring_buffer_free(buffer);}
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:59:11
Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. In many cases it is unclear to say in which state
it is and sometimes it is done a wrong way.
The plan is to convert kthreads into kthread_worker or workqueues
API. It allows to split the functionality into separate operations.
It helps to make a better structure. Also it defines a clean state
where no locks are taken, IRQs blocked, the kthread might sleep
or even be safely migrated.
The kthread worker API is useful when we want to have a dedicated
single thread for the work. It helps to make sure that it is
available when needed. Also it allows a better control, e.g.
define a scheduling priority.
This patch converts hungtaskd() in kthread worker API because
it modifies the priority.
The conversion is pretty straightforward. One iteration of the
main cycle is transferred into a self-queuing delayed kthread work.
We do not longer need to check if it was waken earlier. Instead,
the work timeout is modified when the timeout value is changed.
The user nice value is set from hung_task_init(). Otherwise, we
would need to add an extra init_work.
The patch also handles the error when the kthead worker could not
be crated from some reasons. It was broken before. For example,
wake_up_process would have failed if watchdog_task inclueded an error
code instead of a valid pointer.
Signed-off-by: Petr Mladek <pmladek@suse.com>
CC: linux-watchdog@vger.kernel.org
---
kernel/hung_task.c | 41 +++++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 16 deletions(-)
@@ -41,7 +41,9 @@ int __read_mostly sysctl_hung_task_warnings = 10;staticint__read_mostlydid_panic;-staticstructtask_struct*watchdog_task;+staticstructkthread_worker*watchdog_worker;+staticvoidwatchdog_func(structkthread_work*dummy);+staticDEFINE_DELAYED_KTHREAD_WORK(watchdog_work,watchdog_func);/**Shouldwepanic(andreboot,ifpanic_timeout=isset)whena
@@ -205,7 +207,9 @@ int proc_dohung_task_timeout_secs(struct ctl_table *table, int write,if(ret||!write)gotoout;-wake_up_process(watchdog_task);+if(watchdog_worker)+mod_delayed_kthread_work(watchdog_worker,&watchdog_work,+timeout_jiffies(sysctl_hung_task_timeout_secs));out:returnret;
@@ -222,30 +226,35 @@ EXPORT_SYMBOL_GPL(reset_hung_task_detector);/**kthreadwhichchecksfortasksstuckinDstate*/-staticintwatchdog(void*dummy)+staticvoidwatchdog_func(structkthread_work*dummy){-set_user_nice(current,0);+unsignedlongtimeout=sysctl_hung_task_timeout_secs;-for(;;){-unsignedlongtimeout=sysctl_hung_task_timeout_secs;+if(atomic_xchg(&reset_hung_task,0))+gotonext;-while(schedule_timeout_interruptible(timeout_jiffies(timeout)))-timeout=sysctl_hung_task_timeout_secs;+check_hung_uninterruptible_tasks(timeout);-if(atomic_xchg(&reset_hung_task,0))-continue;--check_hung_uninterruptible_tasks(timeout);-}--return0;+next:+queue_delayed_kthread_work(watchdog_worker,&watchdog_work,+timeout_jiffies(timeout));}staticint__inithung_task_init(void){+structkthread_worker*worker;+atomic_notifier_chain_register(&panic_notifier_list,&panic_block);-watchdog_task=kthread_run(watchdog,NULL,"khungtaskd");+worker=create_kthread_worker(0,"khungtaskd");+if(IS_ERR(worker)){+pr_warn("Failed to create khungtaskd\n");+gotoout;+}+watchdog_worker=worker;+set_user_nice(worker->task,0);+queue_delayed_kthread_work(worker,&watchdog_work,0);+out:return0;}subsys_initcall(hung_task_init);
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:59:23
Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. In many cases it is unclear to say in which state
it is and sometimes it is done a wrong way.
The plan is to convert kthreads into kthread_worker or workqueues
API. It allows to split the functionality into separate operations.
It helps to make a better structure. Also it defines a clean state
where no locks are taken, IRQs blocked, the kthread might sleep
or even be safely migrated.
The kthread worker API is useful when we want to have a dedicated
single thread for the work. It helps to make sure that it is
available when needed. Also it allows a better control, e.g.
define a scheduling priority.
This patch converts the kmemleak kthread into the kthread worker
API because it modifies the scheduling priority.
The result is a simple self-queuing work that just calls kmemleak_scan().
The info messages and set_user_nice() are moved to the functions that
start and stop the worker. These are also renamed to mention worker
instead of thread.
We do not longer need to handle a spurious wakeup and count the remaining
timeout. It is handled by the worker. The delayed work is queued after
the full timeout passes.
Finally, the initial delay is done only when the kthread is started
during the boot. For this we added a parameter to the start function.
Signed-off-by: Petr Mladek <pmladek@suse.com>
CC: Catalin Marinas <catalin.marinas@arm.com>
---
mm/kmemleak.c | 87 +++++++++++++++++++++++++++++------------------------------
1 file changed, 43 insertions(+), 44 deletions(-)
@@ -216,7 +216,8 @@ static int kmemleak_error;staticunsignedlongmin_addr=ULONG_MAX;staticunsignedlongmax_addr;-staticstructtask_struct*scan_thread;+staticstructkthread_worker*kmemleak_scan_worker;+staticstructdelayed_kthread_workkmemleak_scan_work;/* used to avoid reporting of recently allocated objects */staticunsignedlongjiffies_min_age;staticunsignedlongjiffies_last_scan;
@@ -1470,54 +1471,48 @@ static void kmemleak_scan(void)}/*-*Threadfunctionperformingautomaticmemoryscanning.Unreferencedobjects-*attheendofamemoryscanarereportedbutonlythefirsttime.+*Kthreadworkerfunctionperformingautomaticmemoryscanning.+*Unreferencedobjectsattheendofamemoryscanarereported+*butonlythefirsttime.*/-staticintkmemleak_scan_thread(void*arg)+staticvoidkmemleak_scan_func(structkthread_work*dummy){-staticintfirst_run=1;--pr_info("Automatic memory scanning thread started\n");-set_user_nice(current,10);--/*-*Waitbeforethefirstscantoallowthesystemtofullyinitialize.-*/-if(first_run){-first_run=0;-ssleep(SECS_FIRST_SCAN);-}--while(!kthread_should_stop()){-signedlongtimeout=jiffies_scan_wait;--mutex_lock(&scan_mutex);-kmemleak_scan();-mutex_unlock(&scan_mutex);--/* wait before the next scan */-while(timeout&&!kthread_should_stop())-timeout=schedule_timeout_interruptible(timeout);-}--pr_info("Automatic memory scanning thread ended\n");+mutex_lock(&scan_mutex);+kmemleak_scan();+mutex_unlock(&scan_mutex);-return0;+queue_delayed_kthread_work(kmemleak_scan_worker,&kmemleak_scan_work,+jiffies_scan_wait);}/**Starttheautomaticmemoryscanningthread.Thisfunctionmustbecalled*withthescan_mutexheld.*/-staticvoidstart_scan_thread(void)+staticvoidstart_scan_thread(boolboot){-if(scan_thread)+unsignedlongtimeout=0;++if(kmemleak_scan_worker)return;-scan_thread=kthread_run(kmemleak_scan_thread,NULL,"kmemleak");-if(IS_ERR(scan_thread)){-pr_warning("Failed to create the scan thread\n");-scan_thread=NULL;++init_delayed_kthread_work(&kmemleak_scan_work,kmemleak_scan_func);+kmemleak_scan_worker=create_kthread_worker(0,"kmemleak");+if(IS_ERR(kmemleak_scan_worker)){+pr_warn("Failed to create the memory scan worker\n");+kmemleak_scan_worker=NULL;}+pr_info("Automatic memory scanning thread started\n");+set_user_nice(kmemleak_scan_worker->task,10);++/*+*Waitbeforethefirstscantoallowthesystemtofullyinitialize.+*/+if(boot)+timeout=msecs_to_jiffies(SECS_FIRST_SCAN*MSEC_PER_SEC);++queue_delayed_kthread_work(kmemleak_scan_worker,&kmemleak_scan_work,+timeout);}/*
@@ -1962,7 +1961,7 @@ static int __init kmemleak_late_init(void)if(!dentry)pr_warning("Failed to create the debugfs kmemleak file\n");mutex_lock(&scan_mutex);-start_scan_thread();+start_scan_thread(true);mutex_unlock(&scan_mutex);pr_info("Kernel memory leak detector initialized\n");
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:59:34
Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. In many cases it is unclear to say in which state
it is and sometimes it is done a wrong way.
The plan is to convert kthreads into kthread_worker or workqueues
API. It allows to split the functionality into separate operations.
It helps to make a better structure. Also it defines a clean state
where no locks are taken, IRQs blocked, the kthread might sleep
or even be safely migrated.
The kthread worker API is useful when we want to have a dedicated
single thread for the work. It helps to make sure that it is
available when needed. Also it allows a better control, e.g.
define a scheduling priority.
This patch converts kipmi kthread into the kthread worker API because
it modifies the scheduling priority. The change is quite straightforward.
First, we move the per-thread variable "busy_until" into the per-thread
structure struct smi_info. As a side effect, we could omit one parameter
in ipmi_thread_busy_wait(). On the other hand, the structure could not
longer be passed with the const qualifier.
The value of "busy_until" is initialized when the kthread is created.
Also the scheduling priority is set there. This helps to avoid an extra
init work.
One iteration of the kthread cycle is moved to a delayed work function.
The different delays between the cycles are solved the following way:
+ immediate cycle (nope) is converted into goto within the same work
+ immediate cycle with a possible reschedule is converted into
re-queuing with a zero delay
+ schedule_timeout() is converted into re-queuing with the given
delay
+ interruptible sleep is converted into nothing; The work
will get queued again from the check_start_timer_thread().
By other words the external wakeup_up_process() will get
replaced by queuing with a zero delay.
Probably the most tricky change is when the worker is being stopped.
We need to explicitly cancel the work to prevent it from re-queuing.
Signed-off-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Corey Minyard <redacted>
---
drivers/char/ipmi/ipmi_si_intf.c | 121 ++++++++++++++++++++++-----------------
1 file changed, 69 insertions(+), 52 deletions(-)
@@ -304,7 +304,9 @@ struct smi_info {/* Counters and things for the proc filesystem. */atomic_tstats[SI_NUM_STATS];-structtask_struct*thread;+structkthread_worker*worker;+structdelayed_kthread_workwork;+structtimespec64busy_until;structlist_headlink;unionipmi_smi_info_unionaddr_info;
@@ -1032,10 +1035,10 @@ static inline int ipmi_si_is_busy(struct timespec64 *ts)}staticinlineintipmi_thread_busy_wait(enumsi_sm_resultsmi_result,-conststructsmi_info*smi_info,-structtimespec64*busy_until)+structsmi_info*smi_info){unsignedintmax_busy_us=0;+structtimespec64*busy_until=&smi_info->busy_until;if(smi_info->intf_num<num_max_busy_us)max_busy_us=kipmid_max_busy_us[smi_info->intf_num];
@@ -1066,53 +1069,49 @@ static inline int ipmi_thread_busy_wait(enum si_sm_result smi_result,*(ifthatisenabled).Seetheparagraphonkimid_max_busy_usin*Documentation/IPMI.txtfordetails.*/-staticintipmi_thread(void*data)+staticvoidipmi_kthread_worker_func(structkthread_work*work){-structsmi_info*smi_info=data;+structsmi_info*smi_info=container_of(work,structsmi_info,+work.work);unsignedlongflags;enumsi_sm_resultsmi_result;-structtimespec64busy_until;+intbusy_wait;-ipmi_si_set_not_busy(&busy_until);-set_user_nice(current,MAX_NICE);-while(!kthread_should_stop()){-intbusy_wait;+next:+spin_lock_irqsave(&(smi_info->si_lock),flags);+smi_result=smi_event_handler(smi_info,0);-spin_lock_irqsave(&(smi_info->si_lock),flags);-smi_result=smi_event_handler(smi_info,0);+/*+*Ifthedriverisdoingsomething,thereisapossible+*racewiththetimer.Ifthetimerhandlerseeidle,+*andthethreadhereseessomethingelse,thetimer+*handlerwon'trestartthetimereventhoughitis+*required.Sostartithereifnecessary.+*/+if(smi_result!=SI_SM_IDLE&&!smi_info->timer_running)+smi_mod_timer(smi_info,jiffies+SI_TIMEOUT_JIFFIES);-/*-*Ifthedriverisdoingsomething,thereisapossible-*racewiththetimer.Ifthetimerhandlerseeidle,-*andthethreadhereseessomethingelse,thetimer-*handlerwon'trestartthetimereventhoughitis-*required.Sostartithereifnecessary.-*/-if(smi_result!=SI_SM_IDLE&&!smi_info->timer_running)-smi_mod_timer(smi_info,jiffies+SI_TIMEOUT_JIFFIES);--spin_unlock_irqrestore(&(smi_info->si_lock),flags);-busy_wait=ipmi_thread_busy_wait(smi_result,smi_info,-&busy_until);-if(smi_result==SI_SM_CALL_WITHOUT_DELAY)-;/* do nothing */-elseif(smi_result==SI_SM_CALL_WITH_DELAY&&busy_wait)-schedule();-elseif(smi_result==SI_SM_IDLE){-if(atomic_read(&smi_info->need_watch)){-schedule_timeout_interruptible(100);-}else{-/* Wait to be woken up when we are needed. */-__set_current_state(TASK_INTERRUPTIBLE);-schedule();-}-}else-schedule_timeout_interruptible(1);+spin_unlock_irqrestore(&(smi_info->si_lock),flags);+busy_wait=ipmi_thread_busy_wait(smi_result,smi_info);++if(smi_result==SI_SM_CALL_WITHOUT_DELAY)+gotonext;+if(smi_result==SI_SM_CALL_WITH_DELAY&&busy_wait){+queue_delayed_kthread_work(smi_info->worker,+&smi_info->work,0);+}elseif(smi_result==SI_SM_IDLE){+if(atomic_read(&smi_info->need_watch)){+queue_delayed_kthread_work(smi_info->worker,+&smi_info->work,100);+}else{+/* Nope. Wait to be queued when we are needed. */+}+}else{+queue_delayed_kthread_work(smi_info->worker,+&smi_info->work,1);}-return0;}-staticvoidpoll(void*send_info){structsmi_info*smi_info=send_info;
@@ -1253,17 +1252,30 @@ static int smi_start_processing(void *send_info,enable=1;if(enable){-new_smi->thread=kthread_run(ipmi_thread,new_smi,-"kipmi%d",new_smi->intf_num);-if(IS_ERR(new_smi->thread)){+structkthread_worker*worker;++worker=create_kthread_worker(0,"kipmi%d",+new_smi->intf_num);++if(IS_ERR(worker)){dev_notice(new_smi->dev,"Could not start"" kernel thread due to error %ld, only using"" timers to drive the interface\n",-PTR_ERR(new_smi->thread));-new_smi->thread=NULL;+PTR_ERR(worker));+gotoout;}++ipmi_si_set_not_busy(&new_smi->busy_until);+set_user_nice(worker->task,MAX_NICE);++init_delayed_kthread_work(&new_smi->work,+ipmi_kthread_worker_func);+queue_delayed_kthread_work(worker,&new_smi->work,0);++new_smi->worker=worker;}+out:return0;}
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:59:45
Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. In many cases it is unclear to say in which state
it is and sometimes it is done a wrong way.
The plan is to convert kthreads into kthread_worker or workqueues
API. It allows to split the functionality into separate operations.
It helps to make a better structure. Also it defines a clean state
where no locks are taken, IRQs blocked, the kthread might sleep
or even be safely migrated.
The kthread worker API is useful when we want to have a dedicated
single thread for the work. It helps to make sure that it is
available when needed. Also it allows a better control, e.g.
define a scheduling priority.
This patch converts the frm_pool kthread into the kthread worker
API because I am not sure how busy the thread is. It is well
possible that it does not need a dedicated kthread and workqueues
would be perfectly fine. Well, the conversion between kthread
worker API and workqueues is pretty trivial.
The patch moves one iteration from the kthread into the work function.
It preserves the check for a spurious queuing (wake up). Then it
processes one request. Finally, it re-queues itself if more requests
are pending.
Otherwise, wake_up_process() is replaced by queuing the work.
Important: The change is only compile tested. I did not find an easy
way how to check it in a real life.
Signed-off-by: Petr Mladek <pmladek@suse.com>
CC: Doug Ledford <redacted>
CC: Sean Hefty <redacted>
CC: Hal Rosenstock <redacted>
CC: linux-rdma@vger.kernel.org
---
drivers/infiniband/core/fmr_pool.c | 54 ++++++++++++++++++--------------------
1 file changed, 25 insertions(+), 29 deletions(-)
@@ -396,7 +392,7 @@ int ib_flush_fmr_pool(struct ib_fmr_pool *pool)spin_unlock_irq(&pool->pool_lock);serial=atomic_inc_return(&pool->req_ser);-wake_up_process(pool->thread);+queue_kthread_work(pool->worker,&pool->work);if(wait_event_interruptible(pool->force_wait,atomic_read(&pool->flush_ser)-serial>=0))
@@ -510,7 +506,7 @@ int ib_fmr_pool_unmap(struct ib_pool_fmr *fmr)list_add_tail(&fmr->list,&pool->dirty_list);if(++pool->dirty_len>=pool->dirty_watermark){atomic_inc(&pool->req_ser);-wake_up_process(pool->thread);+queue_kthread_work(pool->worker,&pool->work);}}}
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 14:59:55
There is an attempt to print debug messages when the kthread is waken
and when it goes into sleep. It does not work well because the spin lock
does not guard all manipulations with the thread state.
I did not find a way how to print a message when the kthread really
goes into sleep. Instead, I added a state variable. It clearly marks
when a series of IO requests is started and finished. It makes sure
that we always have a pair of started/done messages.
The only problem is that it will print these messages also when
the kthread is created and there is no real work. We might want
to use create_kthread() instead of run_kthread(). Then the kthread
will stay stopped until the first request.
Important: This change is only compile tested. I did not find an easy
way how to test it. This is why I was conservative and did not modify
the kthread creation.
Signed-off-by: Petr Mladek <pmladek@suse.com>
CC: Maxim Levitsky <maximlevitsky@gmail.com>
---
drivers/memstick/host/r592.c | 19 +++++++++----------
drivers/memstick/host/r592.h | 2 +-
2 files changed, 10 insertions(+), 11 deletions(-)
@@ -568,21 +568,24 @@ static int r592_process_thread(void *data){interror;structr592_device*dev=(structr592_device*)data;-unsignedlongflags;while(!kthread_should_stop()){-spin_lock_irqsave(&dev->io_thread_lock,flags);+if(!dev->io_started){+dbg_verbose("IO: started");+dev->io_started=true;+}+set_current_state(TASK_INTERRUPTIBLE);error=memstick_next_req(dev->host,&dev->req);-spin_unlock_irqrestore(&dev->io_thread_lock,flags);if(error){if(error==-ENXIO||error==-EAGAIN){-dbg_verbose("IO: done IO, sleeping");+dbg_verbose("IO: done");}else{dbg("IO: unknown error from ""memstick_next_req %d",error);}+dev->io_started=false;if(kthread_should_stop())set_current_state(TASK_RUNNING);
@@ -714,15 +717,11 @@ static int r592_set_param(struct memstick_host *host,staticvoidr592_submit_req(structmemstick_host*host){structr592_device*dev=memstick_priv(host);-unsignedlongflags;if(dev->req)return;-spin_lock_irqsave(&dev->io_thread_lock,flags);-if(wake_up_process(dev->io_thread))-dbg_verbose("IO thread woken to process requests");-spin_unlock_irqrestore(&dev->io_thread_lock,flags);+wake_up_process(dev->io_thread);}staticconststructpci_device_idr592_pci_id_tbl[]={
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 15:00:11
Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. In many cases it is unclear to say in which state
it is and sometimes it is done a wrong way.
The plan is to convert kthreads into kthread_worker or workqueues
API. It allows to split the functionality into separate operations.
It helps to make a better structure. Also it defines a clean state
where no locks are taken, IRQs blocked, the kthread might sleep
or even be safely migrated.
The kthread worker API is useful when we want to have a dedicated
single thread for the work. It helps to make sure that it is
available when needed. Also it allows a better control, e.g.
define a scheduling priority.
This patch converts the r592_io kthread into the kthread worker
API. I am not sure how busy the kthread is and if anyone would
like to control the resources. It is well possible that a workqueue
would be perfectly fine. Well, the conversion between kthread
worker API and workqueues is pretty trivial.
The patch moves one iteration from the kthread into the kthread
worker function. It helps to remove all the hackery with process
state and kthread_should_stop().
The work is queued instead of waking the thread.
The work is explicitly canceled before the worker is destroyed.
It is self-queuing and it might take a long time until the queue
is drained, otherwise.
Important: The change is only compile tested. I did not find an easy
way how to check it in use.
Signed-off-by: Petr Mladek <pmladek@suse.com>
CC: Maxim Levitsky <maximlevitsky@gmail.com>
---
drivers/memstick/host/r592.c | 58 ++++++++++++++++++++------------------------
drivers/memstick/host/r592.h | 3 ++-
2 files changed, 28 insertions(+), 33 deletions(-)
@@ -563,40 +563,32 @@ out:return;}-/* Main request processing thread */-staticintr592_process_thread(void*data)+/* Main request processing work */+staticvoidr592_process_func(structkthread_work*work){interror;-structr592_device*dev=(structr592_device*)data;--while(!kthread_should_stop()){-if(!dev->io_started){-dbg_verbose("IO: started");-dev->io_started=true;-}--set_current_state(TASK_INTERRUPTIBLE);-error=memstick_next_req(dev->host,&dev->req);+structr592_device*dev=+container_of(work,structr592_device,io_work);-if(error){-if(error==-ENXIO||error==-EAGAIN){-dbg_verbose("IO: done");-}else{-dbg("IO: unknown error from "-"memstick_next_req %d",error);-}-dev->io_started=false;+if(!dev->io_started){+dbg_verbose("IO: started");+dev->io_started=true;+}-if(kthread_should_stop())-set_current_state(TASK_RUNNING);+error=memstick_next_req(dev->host,&dev->req);-schedule();+if(error){+if(error==-ENXIO||error==-EAGAIN){+dbg_verbose("IO: done");}else{-set_current_state(TASK_RUNNING);-r592_execute_tpc(dev);+dbg("IO: unknown error from memstick_next_req %d",+error);}+dev->io_started=false;+}else{+r592_execute_tpc(dev);+queue_kthread_work(dev->io_worker,&dev->io_work);}-return0;}/* Reprogram chip to detect change in card state */
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 15:00:21
This patch removes a code duplication. It does not modify
the functionality.
Signed-off-by: Petr Mladek <pmladek@suse.com>
CC: Zhang Rui <rui.zhang@intel.com>
CC: Eduardo Valentin <edubezval@gmail.com>
CC: Jacob Pan <redacted>
CC: linux-pm@vger.kernel.org
Acked-by: Jacob Pan <redacted>
---
drivers/thermal/intel_powerclamp.c | 45 +++++++++++++++++---------------------
1 file changed, 20 insertions(+), 25 deletions(-)
@@ -505,10 +505,27 @@ static void poll_pkg_cstate(struct work_struct *dummy)schedule_delayed_work(&poll_pkg_cstate_work,HZ);}+staticvoidstart_power_clamp_thread(unsignedlongcpu)+{+structtask_struct**p=per_cpu_ptr(powerclamp_thread,cpu);+structtask_struct*thread;++thread=kthread_create_on_node(clamp_thread,+(void*)cpu,+cpu_to_node(cpu),+"kidle_inject/%ld",cpu);+if(IS_ERR(thread))+return;++/* bind to cpu here */+kthread_bind(thread,cpu);+wake_up_process(thread);+*p=thread;+}+staticintstart_power_clamp(void){unsignedlongcpu;-structtask_struct*thread;/* check if pkg cstate counter is completely 0, abort in this case */if(!has_pkg_state_counter()){
@@ -530,20 +547,7 @@ static int start_power_clamp(void)/* start one thread per online cpu */for_each_online_cpu(cpu){-structtask_struct**p=-per_cpu_ptr(powerclamp_thread,cpu);--thread=kthread_create_on_node(clamp_thread,-(void*)cpu,-cpu_to_node(cpu),-"kidle_inject/%ld",cpu);-/* bind to cpu here */-if(likely(!IS_ERR(thread))){-kthread_bind(thread,cpu);-wake_up_process(thread);-*p=thread;-}-+start_power_clamp_thread(cpu);}put_online_cpus();
@@ -575,7 +579,6 @@ static int powerclamp_cpu_callback(struct notifier_block *nfb,unsignedlongaction,void*hcpu){unsignedlongcpu=(unsignedlong)hcpu;-structtask_struct*thread;structtask_struct**percpu_thread=per_cpu_ptr(powerclamp_thread,cpu);
@@ -584,15 +587,7 @@ static int powerclamp_cpu_callback(struct notifier_block *nfb,switch(action){caseCPU_ONLINE:-thread=kthread_create_on_node(clamp_thread,-(void*)cpu,-cpu_to_node(cpu),-"kidle_inject/%lu",cpu);-if(likely(!IS_ERR(thread))){-kthread_bind(thread,cpu);-wake_up_process(thread);-*percpu_thread=thread;-}+start_power_clamp_thread(cpu);/* prefer BSP as controlling CPU */if(cpu==0){control_cpu=0;
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Petr Mladek <pmladek@suse.com> Date: 2016-02-22 15:00:34
Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. In many cases it is unclear to say in which state
it is and sometimes it is done a wrong way.
The plan is to convert kthreads into kthread_worker or workqueues
API. It allows to split the functionality into separate operations.
It helps to make a better structure. Also it defines a clean state
where no locks are taken, IRQs blocked, the kthread might sleep
or even be safely migrated.
The kthread worker API is useful when we want to have a dedicated
single thread for the work. It helps to make sure that it is
available when needed. Also it allows a better control, e.g.
define a scheduling priority.
This patch converts the intel powerclamp kthreads into the kthread
worker because they need to have a good control over the assigned
CPUs.
IMHO, the most natural way is to split one cycle into two works.
First one does some balancing and let the CPU work normal
way for some time. The second work checks what the CPU has done
in the meantime and put it into C-state to reach the required
idle time ratio. The delay between the two works is achieved
by the delayed kthread work.
The two works have to share some data that used to be local
variables of the single kthread function. This is achieved
by the new per-CPU struct kthread_worker_data. It might look
as a complication. On the other hand, the long original kthread
function was not nice either.
The patch tries to avoid extra init and cleanup works. All the
actions might be done outside the thread. They are moved
to the functions that create or destroy the worker. Especially,
I checked that the timers are assigned to the right CPU.
The two works are queuing each other. It makes it a bit tricky to
break it when we want to stop the worker. We use the global and
per-worker "clamping" variables to make sure that the re-queuing
eventually stops. We also cancel the works to make it faster.
Note that the canceling is not reliable because the handling
of the two variables and queuing is not synchronized via a lock.
But it is not a big deal because it is just an optimization.
The job is stopped faster than before in most cases.
Signed-off-by: Petr Mladek <pmladek@suse.com>
CC: Zhang Rui <rui.zhang@intel.com>
CC: Eduardo Valentin <edubezval@gmail.com>
CC: Jacob Pan <redacted>
CC: linux-pm@vger.kernel.org
Acked-by: Jacob Pan <redacted>
---
drivers/thermal/intel_powerclamp.c | 287 ++++++++++++++++++++++---------------
1 file changed, 168 insertions(+), 119 deletions(-)
@@ -86,11 +86,27 @@ static unsigned int control_cpu; /* The cpu assigned to collect stat and update*/staticboolclamping;+staticconststructsched_paramsparam={+.sched_priority=MAX_USER_RT_PRIO/2,+};+structpowerclamp_worker_data{+structkthread_worker*worker;+structkthread_workbalancing_work;+structdelayed_kthread_workidle_injection_work;+structtimer_listwakeup_timer;+unsignedintcpu;+unsignedintcount;+unsignedintguard;+unsignedintwindow_size_now;+unsignedinttarget_ratio;+unsignedintduration_jiffies;+boolclamping;+};-staticstructtask_struct*__percpu*powerclamp_thread;+staticstructpowerclamp_worker_data*__percpuworker_data;staticstructthermal_cooling_device*cooling_dev;staticunsignedlong*cpu_clamping_mask;/* bit map for tracking per cpu-*clampingthread+*clampingkthreadworker*/staticunsignedintduration;
@@ -368,100 +384,102 @@ static bool powerclamp_adjust_controls(unsigned int target_ratio,returnset_target_ratio+guard<=current_ratio;}-staticintclamp_thread(void*arg)+staticvoidclamp_balancing_func(structkthread_work*work){-intcpunr=(unsignedlong)arg;-DEFINE_TIMER(wakeup_timer,noop_timer,0,0);-staticconststructsched_paramparam={-.sched_priority=MAX_USER_RT_PRIO/2,-};-unsignedintcount=0;-unsignedinttarget_ratio;+structpowerclamp_worker_data*w_data;+intsleeptime;+unsignedlongtarget_jiffies;+unsignedintcompensation;+intinterval;/* jiffies to sleep for each attempt */-set_bit(cpunr,cpu_clamping_mask);-set_freezable();-init_timer_on_stack(&wakeup_timer);-sched_setscheduler(current,SCHED_FIFO,¶m);--while(true==clamping&&!kthread_should_stop()&&-cpu_online(cpunr)){-intsleeptime;-unsignedlongtarget_jiffies;-unsignedintguard;-unsignedintcompensation=0;-intinterval;/* jiffies to sleep for each attempt */-unsignedintduration_jiffies=msecs_to_jiffies(duration);-unsignedintwindow_size_now;--try_to_freeze();-/*-*makesureuserselectedratiodoesnottakeeffectuntil-*thenextround.adjusttarget_ratioifuserhaschanged-*targetsuchthatwecanconvergequickly.-*/-target_ratio=set_target_ratio;-guard=1+target_ratio/20;-window_size_now=window_size;-count++;+w_data=container_of(work,structpowerclamp_worker_data,+balancing_work);-/*-*systemsmayhavedifferentabilitytoenterpackagelevel-*c-states,thusweneedtocompensatetheinjectedidleratio-*toachievetheactualtargetreportedbytheHW.-*/-compensation=get_compensation(target_ratio);-interval=duration_jiffies*100/(target_ratio+compensation);--/* align idle time */-target_jiffies=roundup(jiffies,interval);-sleeptime=target_jiffies-jiffies;-if(sleeptime<=0)-sleeptime=1;-schedule_timeout_interruptible(sleeptime);-/*-*onlyelectedcontrollingcpucancollectstatsandupdate-*controlparameters.-*/-if(cpunr==control_cpu&&!(count%window_size_now)){-should_skip=-powerclamp_adjust_controls(target_ratio,-guard,window_size_now);-smp_mb();-}+/*+*makesureuserselectedratiodoesnottakeeffectuntil+*thenextround.adjusttarget_ratioifuserhaschanged+*targetsuchthatwecanconvergequickly.+*/+w_data->target_ratio=READ_ONCE(set_target_ratio);+w_data->guard=1+w_data->target_ratio/20;+w_data->window_size_now=window_size;+w_data->duration_jiffies=msecs_to_jiffies(duration);+w_data->count++;++/*+*systemsmayhavedifferentabilitytoenterpackagelevel+*c-states,thusweneedtocompensatetheinjectedidleratio+*toachievetheactualtargetreportedbytheHW.+*/+compensation=get_compensation(w_data->target_ratio);+interval=w_data->duration_jiffies*100/+(w_data->target_ratio+compensation);++/* align idle time */+target_jiffies=roundup(jiffies,interval);+sleeptime=target_jiffies-jiffies;+if(sleeptime<=0)+sleeptime=1;++if(clamping&&w_data->clamping&&cpu_online(w_data->cpu))+queue_delayed_kthread_work(w_data->worker,+&w_data->idle_injection_work,+sleeptime);+}++staticvoidclamp_idle_injection_func(structkthread_work*work)+{+structpowerclamp_worker_data*w_data;+unsignedlongtarget_jiffies;++w_data=container_of(work,structpowerclamp_worker_data,+idle_injection_work.work);++/*+*onlyelectedcontrollingcpucancollectstatsandupdate+*controlparameters.+*/+if(w_data->cpu==control_cpu&&+!(w_data->count%w_data->window_size_now)){+should_skip=+powerclamp_adjust_controls(w_data->target_ratio,+w_data->guard,+w_data->window_size_now);+smp_mb();+}-if(should_skip)-continue;+if(should_skip)+gotobalance;++target_jiffies=jiffies+w_data->duration_jiffies;+mod_timer(&w_data->wakeup_timer,target_jiffies);+if(unlikely(local_softirq_pending()))+gotobalance;+/*+*stoptickschedduringidletime,interruptsarestill+*allowed.thusjiffiesareupdatedproperly.+*/+preempt_disable();+/* mwait until target jiffies is reached */+while(time_before(jiffies,target_jiffies)){+unsignedlongecx=1;+unsignedlongeax=target_mwait;-target_jiffies=jiffies+duration_jiffies;-mod_timer(&wakeup_timer,target_jiffies);-if(unlikely(local_softirq_pending()))-continue;/*-*stoptickschedduringidletime,interruptsarestill-*allowed.thusjiffiesareupdatedproperly.+*REVISIT:maycallenter_idle()tonotifydriverswho+*cansavepowerduringcpuidle.sameforexit_idle()*/-preempt_disable();-/* mwait until target jiffies is reached */-while(time_before(jiffies,target_jiffies)){-unsignedlongecx=1;-unsignedlongeax=target_mwait;--/*-*REVISIT:maycallenter_idle()tonotifydriverswho-*cansavepowerduringcpuidle.sameforexit_idle()-*/-local_touch_nmi();-stop_critical_timings();-mwait_idle_with_hints(eax,ecx);-start_critical_timings();-atomic_inc(&idle_wakeup_counter);-}-preempt_enable();+local_touch_nmi();+stop_critical_timings();+mwait_idle_with_hints(eax,ecx);+start_critical_timings();+atomic_inc(&idle_wakeup_counter);}-del_timer_sync(&wakeup_timer);-clear_bit(cpunr,cpu_clamping_mask);+preempt_enable();-return0;+balance:+if(clamping&&w_data->clamping&&cpu_online(w_data->cpu))+queue_kthread_work(w_data->worker,&w_data->balancing_work);}/*
@@ -505,22 +523,58 @@ static void poll_pkg_cstate(struct work_struct *dummy)schedule_delayed_work(&poll_pkg_cstate_work,HZ);}-staticvoidstart_power_clamp_thread(unsignedlongcpu)+staticvoidstart_power_clamp_worker(unsignedlongcpu){-structtask_struct**p=per_cpu_ptr(powerclamp_thread,cpu);-structtask_struct*thread;--thread=kthread_create_on_node(clamp_thread,-(void*)cpu,-cpu_to_node(cpu),-"kidle_inject/%ld",cpu);-if(IS_ERR(thread))+structpowerclamp_worker_data*w_data=per_cpu_ptr(worker_data,cpu);+structkthread_worker*worker;++worker=create_kthread_worker_on_cpu(cpu,KTW_FREEZABLE,+"kidle_inject/%ld");+if(IS_ERR(worker))return;-/* bind to cpu here */-kthread_bind(thread,cpu);-wake_up_process(thread);-*p=thread;+w_data->worker=worker;+w_data->count=0;+w_data->cpu=cpu;+w_data->clamping=true;+set_bit(cpu,cpu_clamping_mask);+setup_timer(&w_data->wakeup_timer,noop_timer,0);+sched_setscheduler(worker->task,SCHED_FIFO,&sparam);+init_kthread_work(&w_data->balancing_work,clamp_balancing_func);+init_delayed_kthread_work(&w_data->idle_injection_work,+clamp_idle_injection_func);+queue_kthread_work(w_data->worker,&w_data->balancing_work);+}++staticvoidstop_power_clamp_worker(unsignedlongcpu)+{+structpowerclamp_worker_data*w_data=per_cpu_ptr(worker_data,cpu);++if(!w_data->worker)+return;++w_data->clamping=false;+/*+*Makesurethatallworksthatgetqueuedafterthispointsee+*theclampingdisabled.Thecounterpartisnotneededbecause+*thereisanimplicitmemorybarrierwhenthequeuedwork+*isproceed.+*/+smp_wmb();+cancel_kthread_work_sync(&w_data->balancing_work);+cancel_delayed_kthread_work_sync(&w_data->idle_injection_work);+/*+*Thebalancingworkstillmightbequeuedherebecause+*thehandlingofthe"clapming"variable,cancel,andqueue+*operationsarenotsynchronizedviaalock.Butitisnot+*abigdeal.Thebalancingworkisfastanddestroykthread+*willwaitforit.+*/+del_timer_sync(&w_data->wakeup_timer);+clear_bit(w_data->cpu,cpu_clamping_mask);+destroy_kthread_worker(w_data->worker);++w_data->worker=NULL;}staticintstart_power_clamp(void)
@@ -545,9 +599,9 @@ static int start_power_clamp(void)clamping=true;schedule_delayed_work(&poll_pkg_cstate_work,0);-/* start one thread per online cpu */+/* start one kthread worker per online cpu */for_each_online_cpu(cpu){-start_power_clamp_thread(cpu);+start_power_clamp_worker(cpu);}put_online_cpus();
@@ -557,20 +611,17 @@ static int start_power_clamp(void)staticvoidend_power_clamp(void){inti;-structtask_struct*thread;-clamping=false;/*-*makeclampingvisibletoothercpusandgivepercpuclampingthreads-*sometimetoexit,orgetskilledlater.+*Blockrequeuinginallthekthreadworkers.Theywilldrainand+*stopfaster.*/-smp_mb();-msleep(20);+clamping=false;if(bitmap_weight(cpu_clamping_mask,num_possible_cpus())){for_each_set_bit(i,cpu_clamping_mask,num_possible_cpus()){-pr_debug("clamping thread for cpu %d alive, kill\n",i);-thread=*per_cpu_ptr(powerclamp_thread,i);-kthread_stop(thread);+pr_debug("clamping worker for cpu %d alive, destroy\n",+i);+stop_power_clamp_worker(i);}}}
@@ -579,15 +630,13 @@ static int powerclamp_cpu_callback(struct notifier_block *nfb,unsignedlongaction,void*hcpu){unsignedlongcpu=(unsignedlong)hcpu;-structtask_struct**percpu_thread=-per_cpu_ptr(powerclamp_thread,cpu);if(false==clamping)gotoexit_ok;switch(action){caseCPU_ONLINE:-start_power_clamp_thread(cpu);+start_power_clamp_worker(cpu);/* prefer BSP as controlling CPU */if(cpu==0){control_cpu=0;
@@ -598,7 +647,7 @@ static int powerclamp_cpu_callback(struct notifier_block *nfb,if(test_bit(cpu,cpu_clamping_mask)){pr_err("cpu %lu dead but powerclamping thread is not\n",cpu);-kthread_stop(*percpu_thread);+stop_power_clamp_worker(cpu);}if(cpu==control_cpu){control_cpu=smp_processor_id();
@@ -785,8 +834,8 @@ static int __init powerclamp_init(void)window_size=2;register_hotcpu_notifier(&powerclamp_cpu_notifier);-powerclamp_thread=alloc_percpu(structtask_struct*);-if(!powerclamp_thread){+worker_data=alloc_percpu(structpowerclamp_worker_data);+if(!worker_data){retval=-ENOMEM;gotoexit_unregister;}
@@ -806,7 +855,7 @@ static int __init powerclamp_init(void)return0;exit_free_thread:-free_percpu(powerclamp_thread);+free_percpu(worker_data);exit_unregister:unregister_hotcpu_notifier(&powerclamp_cpu_notifier);exit_free:
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>