Hello, On Thu, Oct 25, 2012 at 07:34:33PM +0200, Oleg Nesterov wrote: > > I think it would be great if the description is more detailed. This > > code path always makes my head spin and I think we can definitely use > > some more guiding in understanding this dang thing. :) > > Do you mean describe the race in more details? OK, will do and resend > tomorrow. Yeah and maybe explain briefly how schedule_freezable() gets us out of the trouble. > > > @@ -2092,7 +2085,7 @@ static bool do_signal_stop(int signr) > > > } > > > > > > /* Now we don't run again until woken by SIGCONT or SIGKILL */ > > > - schedule(); > > > + freezable_schedule(); > > > > This makes me wonder whether we still need try_to_freeze() in > > get_signal_to_deliver() right after the relock: label. Freezer no > > longer treats STOPPED/TRACED special and both sleeping sites in signal > > deliver path are marked freezable_schedule(). We shouldn't need the > > explicit try_to_freeze(), right? > > OOPS. > > I'd say this doesn't really matter but yes we can move it up, > get_signal_to_deliver() will be called again. Right, we can't remove it. That's our main freezing point for userland tasks. > But! the comment above try_to_freeze() becomes misleading with > this patch, so this really needs v2. But, yeah, I think we should move it above relock: and update the comment to explain that that's the usual freezing site. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
