On 4/16/2015 11:28 AM, Frederic Weisbecker wrote:
+ /* Unpark any threads that were voluntarily parked. */ >+ if (ht->cpumask) { >+ cpumask_andnot(&tmp_mask, cpu_online_mask, ht->cpumask); >+ for_each_cpu(cpu, &tmp_mask) { >+ struct task_struct *tsk = *per_cpu_ptr(ht->store, cpu); >+ if (tsk) >+ kthread_unpark(tsk); >+ } >+ }Why do you need to do that? smpboot_destroy_threads() doesn't work on parked threads? But kthread_stop() does an explicit unparking.
Yes, this part left me scratching my head. Experimentally, this was necessary. I saw the unpark in kthread_stop() but it didn't make things work properly. Currently it looks like parked threads are only in that state while cores are being offlined, and then they are killed individually, so it seems likely that this particular path hasn't been tested before.
+/* Statically allocated and used under smpboot_threads_lock. */ +static struct cpumask tmp_mask; + Better allocate the cpumask on need rather than have it resident on memory. struct cpumask can be large. Plus we need to worry about locking it.
I was trying to avoid the need to make functions return errors for the extremely unlikely case of ENOMEM. No one is going to check that error return in practice anyway; programmers are lazy. It seemed easy to allocate one mask statically and use it under the lock; even large systems aren't likely to burn more than a couple hundred bytes of .bss for this. But, if you'd prefer using allocation and the error-return model, I can certainly change the code to do that. -- Chris Metcalf, EZChip Semiconductor http://www.ezchip.com -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
