On 04/05, Gautham R Shenoy wrote: > > > > @@ -147,6 +147,7 @@ cpu_callback(struct notifier_block *nfb, > > > case CPU_DEAD: > > > p = per_cpu(watchdog_task, hotcpu); > > > per_cpu(watchdog_task, hotcpu) = NULL; > > > + thaw_process(p); > > > kthread_stop(p); > > > > As it was already discussed, this is racy. As Srivatsa (imho rightly) > > suggested, kthread_stop(p) should thaw process itself. This also allows > > us to kill at least some of wait_for_die loops. > > > > Well, in this case this is not racy. Remember, we're doing a > thaw_process(p) in CPU_DEAD where p *is* frozen for cpu hotplug. So > the where we might call a freeze_process(p) after we do a thaw_process > doesn't seem to be feasible.
Oops, yes. > > However, the change in kthread_stop(p) in not enough to close the race. > > We can check kthread_should_stop() in refrigerator(), this looks like > > a most simple approach for now. > > > > Why the check kthread_should_stop() refrigerator() ? > As vatsa mentioned, we would be doing > > task_lock(p); > freezer_exempt(p, FE_ALL); /* Doesn't exist as of now, but we can work > it out */ > thaw_process(p); > task_unlock(p); Please look at http://marc.info/?l=linux-kernel&m=117562018530190, we can't change p->flags unless we know it is frozen. > > Alternatively, Srivatsa suggests to introduce a new task_lock() protected > > task_struct->freezer_state (so we can reliably set FE_ALL). Surely this is > > more poweful, but needs more changes. I am not sure. Perhaps we can do > > this later. > > This needs an extra field! We're supposed to be miserly when it comes to > adding new fields to task_struct, now aren't we :-) That is why "Perhaps we can do this later" :) > > In any case, imho "try3" should add thaw_process() to kthread_stop(). > > Gautham, Srivatsa, do you agree? > > > > Completely. Working on it now. Great! Oleg. - 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/
