From: Tejun Heo <tj@kernel.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: torvalds@linux-foundation.org, mingo@elte.hu,
peterz@infradead.org, awalls@radix.net,
linux-kernel@vger.kernel.org, jeff@garzik.org,
akpm@linux-foundation.org, jens.axboe@oracle.com,
rusty@rustcorp.com.au, cl@linux-foundation.org,
dhowells@redhat.com, arjan@linux.intel.com, avi@redhat.com,
johannes@sipsolutions.net, andi@firstfloor.org
Subject: Re: [PATCH 16/43] workqueue: kill cpu_populated_map
Date: Tue, 02 Mar 2010 00:32:18 +0900 [thread overview]
Message-ID: <4B8BDE02.1050208@kernel.org> (raw)
In-Reply-To: <20100228160005.GA16144@redhat.com>
Hello,
On 03/01/2010 01:00 AM, Oleg Nesterov wrote:
> On 02/26, Tejun Heo wrote:
>>
>> @@ -1023,41 +991,40 @@ struct workqueue_struct *__create_workqueue_key(const char *name,
>> ...
>> + cpu_maps_update_done();
>> ...
>> +
>> + spin_lock(&workqueue_lock);
>> + list_add(&wq->list, &workqueues);
>> + spin_unlock(&workqueue_lock);
>
> OK, but if cpu_up() happens right after we drop cpu_maps_update_done(),
> cwq->thread on the new CPU will run unbound?
Indeed, looks like I was too impatient with cpu_maps_update_done().
>> @@ -1127,47 +1091,30 @@ static int __devinit workqueue_cpu_callback(struct notifier_block *nfb,
>> ...
>> list_for_each_entry(wq, &workqueues, list) {
>
> this becomes unsafe. create/destroy can modify workqueues list
> in parallel.
Yeap, has always been like that. Will be fixed by later changes.
>> case CPU_ONLINE:
>> - start_workqueue_thread(cwq, cpu);
>> + __set_cpus_allowed(cwq->thread, get_cpu_mask(cpu),
>> + true);
>
> if the thread doesn't have PF_THREAD_BOUND, who will set it?
Indeed will update the worker function to set PF_THREAD_BOUND itself
but again this problem is gone with later patches.
>> case CPU_POST_DEAD:
>> - cleanup_workqueue_thread(cwq);
>> + lock_map_acquire(&cwq->wq->lockdep_map);
>> + lock_map_release(&cwq->wq->lockdep_map);
>> + flush_cpu_workqueue(cwq);
>
> This can race with destroy_workqueue(), no?
Yes, it can and, again, has been always like that and will be fixed by
later patches.
> I guess this patch is preparation, probably these problems should
> go away later...
I'll fix the new problems but leave the existing ones alone. I don't
think it's worth fixing them at this point with all the pending
changes.
Thanks.
--
tejun
next prev parent reply other threads:[~2010-03-01 15:33 UTC|newest]
Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-26 12:22 [PATCHSET] workqueue: concurrency managed workqueue, take#4 Tejun Heo
2010-02-26 12:22 ` [PATCH 01/43] sched: consult online mask instead of active in select_fallback_rq() Tejun Heo
2010-02-26 12:22 ` [PATCH 02/43] sched: rename preempt_notifiers to sched_notifiers and refactor implementation Tejun Heo
2010-02-26 12:22 ` [PATCH 03/43] sched: refactor try_to_wake_up() Tejun Heo
2010-02-26 12:22 ` [PATCH 04/43] sched: implement __set_cpus_allowed() Tejun Heo
2010-02-26 12:22 ` [PATCH 05/43] sched: make sched_notifiers unconditional Tejun Heo
2010-02-26 12:22 ` [PATCH 06/43] sched: add wakeup/sleep sched_notifiers and allow NULL notifier ops Tejun Heo
2010-02-26 12:22 ` [PATCH 07/43] sched: implement try_to_wake_up_local() Tejun Heo
2010-02-28 12:33 ` Oleg Nesterov
2010-03-01 14:22 ` Tejun Heo
2010-02-26 12:22 ` [PATCH 08/43] workqueue: change cancel_work_sync() to clear work->data Tejun Heo
2010-02-26 12:22 ` [PATCH 09/43] acpi: use queue_work_on() instead of binding workqueue worker to cpu0 Tejun Heo
2010-02-26 12:22 ` [PATCH 10/43] stop_machine: reimplement without using workqueue Tejun Heo
2010-02-28 14:11 ` Oleg Nesterov
2010-03-01 15:07 ` Tejun Heo
2010-03-01 15:37 ` Oleg Nesterov
2010-03-01 16:36 ` Tejun Heo
2010-03-01 16:50 ` Oleg Nesterov
2010-03-01 18:02 ` Tejun Heo
2010-02-28 14:34 ` Oleg Nesterov
2010-03-01 15:11 ` Tejun Heo
2010-03-01 15:41 ` Oleg Nesterov
2010-02-26 12:22 ` [PATCH 11/43] workqueue: misc/cosmetic updates Tejun Heo
2010-02-26 12:22 ` [PATCH 12/43] workqueue: merge feature parameters into flags Tejun Heo
2010-02-26 12:22 ` [PATCH 13/43] workqueue: define masks for work flags and conditionalize STATIC flags Tejun Heo
2010-02-26 12:22 ` [PATCH 14/43] workqueue: separate out process_one_work() Tejun Heo
2010-02-26 12:22 ` [PATCH 15/43] workqueue: temporarily disable workqueue tracing Tejun Heo
2010-02-26 12:22 ` [PATCH 16/43] workqueue: kill cpu_populated_map Tejun Heo
2010-02-28 16:00 ` Oleg Nesterov
2010-03-01 15:32 ` Tejun Heo [this message]
2010-03-01 15:50 ` Oleg Nesterov
2010-03-01 16:19 ` Tejun Heo
2010-02-26 12:22 ` [PATCH 17/43] workqueue: update cwq alignement Tejun Heo
2010-02-28 17:12 ` Oleg Nesterov
2010-03-01 16:40 ` Tejun Heo
2010-02-26 12:22 ` [PATCH 18/43] workqueue: reimplement workqueue flushing using color coded works Tejun Heo
2010-02-28 20:31 ` Oleg Nesterov
2010-03-01 17:33 ` Tejun Heo
2010-03-01 19:47 ` Oleg Nesterov
2010-02-26 12:22 ` [PATCH 19/43] workqueue: introduce worker Tejun Heo
2010-02-26 12:22 ` [PATCH 20/43] workqueue: reimplement work flushing using linked works Tejun Heo
2010-03-01 14:53 ` Oleg Nesterov
2010-03-01 18:00 ` Tejun Heo
2010-03-01 18:51 ` Oleg Nesterov
2010-02-26 12:22 ` [PATCH 21/43] workqueue: implement per-cwq active work limit Tejun Heo
2010-02-26 12:22 ` [PATCH 22/43] workqueue: reimplement workqueue freeze using max_active Tejun Heo
2010-02-26 12:23 ` [PATCH 23/43] workqueue: introduce global cwq and unify cwq locks Tejun Heo
2010-02-26 12:23 ` [PATCH 24/43] workqueue: implement worker states Tejun Heo
2010-02-26 12:23 ` [PATCH 25/43] workqueue: reimplement CPU hotplugging support using trustee Tejun Heo
2010-02-26 12:23 ` [PATCH 26/43] workqueue: make single thread workqueue shared worker pool friendly Tejun Heo
2010-02-26 12:23 ` [PATCH 27/43] workqueue: add find_worker_executing_work() and track current_cwq Tejun Heo
2010-02-26 12:23 ` [PATCH 28/43] workqueue: carry cpu number in work data once execution starts Tejun Heo
2010-02-28 7:08 ` [PATCH UPDATED " Tejun Heo
2010-02-26 12:23 ` [PATCH 29/43] workqueue: implement WQ_NON_REENTRANT Tejun Heo
2010-02-26 12:23 ` [PATCH 30/43] workqueue: use shared worklist and pool all workers per cpu Tejun Heo
2010-02-26 12:23 ` [PATCH 31/43] workqueue: implement concurrency managed dynamic worker pool Tejun Heo
2010-02-26 12:23 ` [PATCH 32/43] workqueue: increase max_active of keventd and kill current_is_keventd() Tejun Heo
2010-02-26 12:23 ` [PATCH 33/43] workqueue: add system_wq, system_long_wq and system_nrt_wq Tejun Heo
2010-02-26 12:23 ` [PATCH 34/43] workqueue: implement DEBUGFS/workqueue Tejun Heo
2010-02-28 7:13 ` [PATCH UPDATED " Tejun Heo
2010-02-26 12:23 ` [PATCH 35/43] workqueue: implement several utility APIs Tejun Heo
2010-02-28 7:15 ` [PATCH UPDATED " Tejun Heo
2010-02-26 12:23 ` [PATCH 36/43] libata: take advantage of cmwq and remove concurrency limitations Tejun Heo
2010-02-26 12:23 ` [PATCH 37/43] async: use workqueue for worker pool Tejun Heo
2010-02-26 12:23 ` [PATCH 38/43] fscache: convert object to use workqueue instead of slow-work Tejun Heo
2010-02-26 12:23 ` [PATCH 39/43] fscache: convert operation " Tejun Heo
2010-02-26 12:23 ` [PATCH 40/43] fscache: drop references to slow-work Tejun Heo
2010-02-26 12:23 ` [PATCH 41/43] cifs: use workqueue instead of slow-work Tejun Heo
2010-02-28 7:09 ` [PATCH UPDATED " Tejun Heo
2010-02-26 12:23 ` [PATCH 42/43] gfs2: " Tejun Heo
2010-02-28 7:10 ` [PATCH UPDATED " Tejun Heo
2010-02-26 12:23 ` [PATCH 43/43] slow-work: kill it Tejun Heo
2010-02-27 22:52 ` [PATCH] workqueue: Fix build on PowerPC Anton Blanchard
2010-02-28 6:08 ` Tejun Heo
2010-02-28 1:00 ` [PATCH] workqueue: Fix a compile warning in work_busy Anton Blanchard
2010-02-28 6:18 ` Tejun Heo
2010-02-28 1:11 ` [PATCHSET] workqueue: concurrency managed workqueue, take#4 Anton Blanchard
2010-02-28 6:32 ` Tejun Heo
2010-03-10 14:52 ` David Howells
2010-03-12 5:03 ` Tejun Heo
2010-03-12 11:23 ` David Howells
2010-03-12 22:55 ` Tejun Heo
2010-03-16 14:38 ` David Howells
2010-03-16 16:03 ` Tejun Heo
2010-03-16 17:18 ` David Howells
2010-04-25 8:09 ` [PATCHSET UPDATED] " Tejun Heo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B8BDE02.1050208@kernel.org \
--to=tj@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=andi@firstfloor.org \
--cc=arjan@linux.intel.com \
--cc=avi@redhat.com \
--cc=awalls@radix.net \
--cc=cl@linux-foundation.org \
--cc=dhowells@redhat.com \
--cc=jeff@garzik.org \
--cc=jens.axboe@oracle.com \
--cc=johannes@sipsolutions.net \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=rusty@rustcorp.com.au \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox