On Tue, 2019-09-17 at 18:00 +0200, Sebastian Andrzej Siewior wrote: > On 2019-07-27 00:56:37 [-0500], Scott Wood wrote: > > migrate_enable() currently open-codes a variant of select_fallback_rq(). > > However, it does not have the "No more Mr. Nice Guy" fallback and thus > > it will pass an invalid CPU to the migration thread if cpus_mask only > > contains a CPU that is !active. > > > > Signed-off-by: Scott Wood <swood@xxxxxxxxxx> > > --- > > This scenario will be more likely after the next patch, since > > the migrate_disable_update check goes away. However, it could happen > > anyway if cpus_mask was updated to a CPU other than the one we were > > pinned to, and that CPU subsequently became inactive. > > I'm unclear about the problem / side effect this has (before and after > the change). It is possible (before and after that change) that a CPU is > selected which is invalid / goes offline after the "preempt_enable()" > statement and before stop_one_cpu() does its job, correct? By "pass an invalid CPU" I don't mean offline; I mean >= nr_cpu_ids which is what cpumask_any_and() returns if the sets don't intersect (a CPU going offline is merely a way to end up in that situation). At one point I observed that causing a crash. I guess is_cpu_allowed() returned true by chance based on the contents of data beyond the end of the cpumask? That doesn't seem likely based on what comes after p->cpus_mask (at that point migrate_disable should be zero), but maybe I had something else at that point in the struct while developing. In any case, not something to be relied on. :-) Going offline after selection shouldn't be a problem. migration_cpu_stop() won't do anything if is_cpu_allowed() returns false, and we'll get migrated off the CPU by migrate_tasks(). Even if we get migrated after reading task_cpu(p) but before queuing the stop machine work, it'll either get flushed when the stopper thread parks, or rejected due to stopper->enabled being false. -Scott
