lkml.org 
[lkml]   [2010]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 16/43] workqueue: kill cpu_populated_map
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


\
 
 \ /
  Last update: 2010-03-01 16:35    [W:0.338 / U:0.300 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site