2013-07-25 10:48:31

by Lai Jiangshan

[permalink] [raw]
Subject: [PATCH] workqueue: clear workers of a pool after the CPU is offline

The unbound pools and their workers can be destroyed/cleared
when their refcnt become zero. But the cpu pool can't be destroyed
due to they are always referenced, their refcnt are always > 0.

We don't want to destroy the cpu pools, but we want to destroy
the workers of the pool when the pool is full idle after the cpu
is offline. This is the default behavior in old days until
we removed the trustee_thread().

We need to find a new way to restore this behavior,
We add offline_pool() and POOL_OFFLINE flag to do so.

1) Before we try to clear workers, we set the POOL_OFFLINE to the pool,
and pool will not serve to works, any work which is tried to be queued
on that pool will be rejected except chained works.

2) when all the pending works are finished and all workers are idle, worker
thread will schedule offline_pool() to clear workers.

Signed-off-by: Lai Jiangshan <[email protected]>
---
kernel/workqueue.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index f02c4a4..2617895 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -63,13 +63,18 @@ enum {
* %WORKER_UNBOUND set and concurrency management disabled, and may
* be executing on any CPU. The pool behaves as an unbound one.
*
- * Note that DISASSOCIATED should be flipped only while holding
- * manager_mutex to avoid changing binding state while
+ * OFFLINE is a further state of DISASSOCIATED when the cpu had
+ * finished offline and all the workers will exit after they
+ * finish the last works of the pool.
+ *
+ * Note that DISASSOCIATED and OFFLINE should be flipped only while
+ * holding manager_mutex to avoid changing binding state while
* create_worker() is in progress.
*/
POOL_MANAGE_WORKERS = 1 << 0, /* need to manage workers */
- POOL_DISASSOCIATED = 1 << 2, /* cpu can't serve workers */
+ POOL_DISASSOCIATED = 1 << 2, /* pool dissociates its cpu */
POOL_FREEZING = 1 << 3, /* freeze in progress */
+ POOL_OFFLINE = 1 << 4, /* pool can't serve work */

/* worker flags */
WORKER_STARTED = 1 << 0, /* started */
@@ -164,6 +169,7 @@ struct worker_pool {
struct mutex manager_arb; /* manager arbitration */
struct mutex manager_mutex; /* manager exclusion */
struct idr worker_idr; /* MG: worker IDs and iteration */
+ struct work_struct offline_work; /* offline the pool */

struct workqueue_attrs *attrs; /* I: worker attributes */
struct hlist_node hash_node; /* PL: unbound_pool_hash node */
@@ -1372,6 +1378,12 @@ retry:
wq->name, cpu);
}

+ if (unlikely(pwq->pool->flags & POOL_OFFLINE) &&
+ WARN_ON_ONCE(!is_chained_work(wq))) {
+ spin_unlock(&pwq->pool->lock);
+ return;
+ }
+
/* pwq determined, queue */
trace_workqueue_queue_work(req_cpu, pwq, work);

@@ -1784,7 +1796,7 @@ static void start_worker(struct worker *worker)
}

/**
- * create_and_start_worker - create and start a worker for a pool
+ * create_and_start_worker - create and start the initial worker for a pool
* @pool: the target pool
*
* Grab the managership of @pool and create and start a new worker for it.
@@ -1798,6 +1810,7 @@ static int create_and_start_worker(struct worker_pool *pool)
worker = create_worker(pool);
if (worker) {
spin_lock_irq(&pool->lock);
+ pool->flags &= ~POOL_OFFLINE;
start_worker(worker);
spin_unlock_irq(&pool->lock);
}
@@ -2091,6 +2104,54 @@ static bool manage_workers(struct worker *worker)
}

/**
+ * offline_pool - try to offline a pool
+ * @work: embedded offline work item of the target pool
+ *
+ * Try to offline a pool by destroying all its workers.
+ *
+ * offline_pool() only destroys workers which are idle on the idle_list.
+ * If any worker leaves idle by some reasons, it can not be destroyed,
+ * but this work item will be rescheduled by the worker's worker_thread()
+ * again in this case. So offline_pool() may be called multi times
+ * to finish offline pool in this rare case.
+ *
+ * offline_pool() is always scheduled by system_unbound_wq even the pool
+ * is high priority pool:
+ * 1) The pool of system_unbound_wq is always online.
+ * 2) The latency of offline_pool() doesn't matter.
+ */
+static void offline_pool(struct work_struct *work)
+{
+ struct worker_pool *pool;
+ struct worker *worker;
+
+ pool = container_of(work, struct worker_pool, offline_work);
+
+ mutex_lock(&pool->manager_mutex);
+ if (!(pool->flags & POOL_OFFLINE)) {
+ /* the pool is back online, cancel offline */
+ mutex_unlock(&pool->manager_mutex);
+ return;
+ }
+
+ spin_lock_irq(&pool->lock);
+ while (!list_empty(&pool->idle_list)) {
+ worker = list_first_entry(&pool->idle_list,
+ struct worker, entry);
+ destroy_worker(worker);
+ }
+ spin_unlock_irq(&pool->lock);
+
+ mutex_unlock(&pool->manager_mutex);
+}
+
+static inline bool need_to_offline_pool(struct worker_pool *pool)
+{
+ return (pool->flags & POOL_OFFLINE) &&
+ (pool->nr_workers == pool->nr_idle);
+}
+
+/**
* process_one_work - process single work
* @worker: self
* @work: work to process
@@ -2251,6 +2312,7 @@ static int worker_thread(void *__worker)
{
struct worker *worker = __worker;
struct worker_pool *pool = worker->pool;
+ bool pool_offline;

/* tell the scheduler that this is a workqueue worker */
worker->task->flags |= PF_WQ_WORKER;
@@ -2320,8 +2382,11 @@ sleep:
* event.
*/
worker_enter_idle(worker);
+ pool_offline = need_to_offline_pool(pool);
__set_current_state(TASK_INTERRUPTIBLE);
spin_unlock_irq(&pool->lock);
+ if (pool_offline)
+ queue_work(system_unbound_wq, &pool->offline_work);
schedule();
goto woke_up;
}
@@ -3451,6 +3516,7 @@ static int init_worker_pool(struct worker_pool *pool)
pool->cpu = -1;
pool->node = NUMA_NO_NODE;
pool->flags |= POOL_DISASSOCIATED;
+ pool->flags |= POOL_OFFLINE;
INIT_LIST_HEAD(&pool->worklist);
INIT_LIST_HEAD(&pool->idle_list);
hash_init(pool->busy_hash);
@@ -3465,6 +3531,7 @@ static int init_worker_pool(struct worker_pool *pool)
mutex_init(&pool->manager_arb);
mutex_init(&pool->manager_mutex);
idr_init(&pool->worker_idr);
+ INIT_WORK(&pool->offline_work, offline_pool);

INIT_HLIST_NODE(&pool->hash_node);
pool->refcnt = 1;
@@ -4702,6 +4769,7 @@ static int __cpuinit workqueue_cpu_down_callback(struct notifier_block *nfb,
void *hcpu)
{
int cpu = (unsigned long)hcpu;
+ struct worker_pool *pool;
struct work_struct unbind_work;
struct workqueue_struct *wq;

@@ -4720,6 +4788,19 @@ static int __cpuinit workqueue_cpu_down_callback(struct notifier_block *nfb,
/* wait for per-cpu unbinding to finish */
flush_work(&unbind_work);
break;
+ case CPU_UP_CANCELED:
+ case CPU_POST_DEAD:
+ for_each_cpu_worker_pool(pool, cpu) {
+ mutex_lock(&pool->manager_mutex);
+ spin_lock_irq(&pool->lock);
+
+ pool->flags |= POOL_OFFLINE;
+ wake_up_worker(pool);
+
+ spin_unlock_irq(&pool->lock);
+ mutex_unlock(&pool->manager_mutex);
+ }
+ break;
}
return NOTIFY_OK;
}
--
1.7.4.4


2013-07-25 15:31:32

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH] workqueue: clear workers of a pool after the CPU is offline

Hello, Lai.

On Thu, Jul 25, 2013 at 06:52:02PM +0800, Lai Jiangshan wrote:
> The unbound pools and their workers can be destroyed/cleared
> when their refcnt become zero. But the cpu pool can't be destroyed
> due to they are always referenced, their refcnt are always > 0.
>
> We don't want to destroy the cpu pools, but we want to destroy
> the workers of the pool when the pool is full idle after the cpu
> is offline. This is the default behavior in old days until
> we removed the trustee_thread().
>
> We need to find a new way to restore this behavior,
> We add offline_pool() and POOL_OFFLINE flag to do so.

Hmmm... if I'm not confused, now the cpu pools just behave like a
normal unbound pool when the cpu goes down, which means that the idle
cpu workers will exit once idle timeout is reached, right? I really
don't think it'd be worthwhile to add extra logic to accelerate the
process.

Note that there actually are benefits to doing it asynchronously as
CPUs go up and down very frequently on mobile platforms and destroying
idle workers as soon as possible would just mean that we'd be doing a
lot of work which isn't necessary. I mean, we even grew an explicit
mechanism to park kthreads to avoid repeatedly creating and destroying
per-cpu kthreads as cpus go up and down. I don't see any point in
adding code to go the other direction.

Thanks.

--
tejun

2013-07-26 02:09:39

by Lai Jiangshan

[permalink] [raw]
Subject: Re: [PATCH] workqueue: clear workers of a pool after the CPU is offline

On 07/25/2013 11:31 PM, Tejun Heo wrote:
> Hello, Lai.
>
> On Thu, Jul 25, 2013 at 06:52:02PM +0800, Lai Jiangshan wrote:
>> The unbound pools and their workers can be destroyed/cleared
>> when their refcnt become zero. But the cpu pool can't be destroyed
>> due to they are always referenced, their refcnt are always > 0.
>>
>> We don't want to destroy the cpu pools, but we want to destroy
>> the workers of the pool when the pool is full idle after the cpu
>> is offline. This is the default behavior in old days until
>> we removed the trustee_thread().
>>
>> We need to find a new way to restore this behavior,
>> We add offline_pool() and POOL_OFFLINE flag to do so.
>
> Hmmm... if I'm not confused, now the cpu pools just behave like a
> normal unbound pool when the cpu goes down,

cpu pools are always referenced, they don't behave like unbound pool.

> which means that the idle
> cpu workers will exit once idle timeout is reached, right?

No, no code to force the cpu workers quit currently.
you can just offline a cpu to see what happened to the workers.

> I really
> don't think it'd be worthwhile to add extra logic to accelerate the
> process.
>
> Note that there actually are benefits to doing it asynchronously as
> CPUs go up and down very frequently on mobile platforms and destroying
> idle workers as soon as possible would just mean that we'd be doing a
> lot of work which isn't necessary. I mean, we even grew an explicit
> mechanism to park kthreads to avoid repeatedly creating and destroying
> per-cpu kthreads as cpus go up and down. I don't see any point in
> adding code to go the other direction.
>
> Thanks.
>

2013-07-26 03:07:22

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH] workqueue: clear workers of a pool after the CPU is offline

Hello,

On Fri, Jul 26, 2013 at 10:13:25AM +0800, Lai Jiangshan wrote:
> > Hmmm... if I'm not confused, now the cpu pools just behave like a
> > normal unbound pool when the cpu goes down,
>
> cpu pools are always referenced, they don't behave like unbound pool.

Yeah sure, they don't get destroyed but pool management functions the
same.

> > which means that the idle
> > cpu workers will exit once idle timeout is reached, right?
>
> No, no code to force the cpu workers quit currently.
> you can just offline a cpu to see what happened to the workers.

Hmmm? The idle timer thing doesn't work? Why?

--
tejun

2013-07-26 03:43:17

by Lai Jiangshan

[permalink] [raw]
Subject: Re: [PATCH] workqueue: clear workers of a pool after the CPU is offline

On 07/26/2013 11:07 AM, Tejun Heo wrote:
> Hello,
>
> On Fri, Jul 26, 2013 at 10:13:25AM +0800, Lai Jiangshan wrote:
>>> Hmmm... if I'm not confused, now the cpu pools just behave like a
>>> normal unbound pool when the cpu goes down,
>>
>> cpu pools are always referenced, they don't behave like unbound pool.
>
> Yeah sure, they don't get destroyed but pool management functions the
> same.
>
>>> which means that the idle
>>> cpu workers will exit once idle timeout is reached, right?
>>
>> No, no code to force the cpu workers quit currently.
>> you can just offline a cpu to see what happened to the workers.
>
> Hmmm? The idle timer thing doesn't work? Why?
>

any worker can't kill itself.
managers always tries to leave 2 workers.

so the workers of the offline cpu pool can't be totally destroyed.

(In old days, we also have idle timer, but the last workers are killed by trustee_thread())

2013-07-26 10:22:45

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH] workqueue: clear workers of a pool after the CPU is offline

On Fri, Jul 26, 2013 at 11:47:04AM +0800, Lai Jiangshan wrote:
> any worker can't kill itself.
> managers always tries to leave 2 workers.
>
> so the workers of the offline cpu pool can't be totally destroyed.

But we *do* want to keep them around as CPUs taken offline are likely
to come online at some point and destroying all of them saves only
~16k of memory while adding more work while CPUs are on/offlined which
can be very frequent on mobile devices. The change was *intentional*.

Thanks.

--
tejun

2013-07-26 16:55:17

by Lai Jiangshan

[permalink] [raw]
Subject: Re: [PATCH] workqueue: clear workers of a pool after the CPU is offline

On Fri, Jul 26, 2013 at 6:22 PM, Tejun Heo <[email protected]> wrote:
> On Fri, Jul 26, 2013 at 11:47:04AM +0800, Lai Jiangshan wrote:
>> any worker can't kill itself.
>> managers always tries to leave 2 workers.
>>
>> so the workers of the offline cpu pool can't be totally destroyed.
>
> But we *do* want to keep them around as CPUs taken offline are likely
> to come online at some point and destroying all of them saves only
> ~16k of memory while adding more work while CPUs are on/offlined which

4 threads, (normal and high priority wq)
~32k
it is still small.

> can be very frequent on mobile devices. The change was *intentional*.

but sometimes the cpu is offline for long time.
and maybe the adminstrator want to reclaim the resource..

Add a boot option or sysfs switch?

>
> Thanks.
>
> --
> tejun
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2013-07-26 17:03:22

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH] workqueue: clear workers of a pool after the CPU is offline

On Sat, Jul 27, 2013 at 12:55:12AM +0800, Lai Jiangshan wrote:
> but sometimes the cpu is offline for long time.
> and maybe the adminstrator want to reclaim the resource..
>
> Add a boot option or sysfs switch?

No, we don't do that to save 64k.

--
tejun