Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On 10/27, Ben Hutchings wrote: > > On Fri, 2012-10-26 at 19:46 +0200, Oleg Nesterov wrote: > > try_to_freeze_tasks() and cgroup_freezer rely on scheduler locks > > to ensure that a task doing STOPPED/TRACED -> RUNNING transition > > can't escape freezing. This mostly works, but ptrace_stop() does > > not necessarily call schedule(), it can change task->state back to > > RUNNING and check freezing() without any lock/barrier in between. > > > > We could add the necessary barrier, but this patch changes > > ptrace_stop() and do_signal_stop() to use freezable_schedule(). > > This fixes the race, freezer_count() and freezer_should_skip() > > carefully avoid the race. > > > > And this simplifies the code, try_to_freeze_tasks/update_if_frozen > > no longer need to use task_is_stopped_or_traced() checks with the > > non trivial assumptions. We can rely on the mechanism which was > > specially designed to mark the sleeping task as "frozen enough". > > > > v2: As Tejun pointed out, we can also change get_signal_to_deliver() > > and move try_to_freeze() up before 'relock' label. > > > > Signed-off-by: Oleg Nesterov > [...] > > This is not the correct way to submit a change to stable. Please see > Documentation/stable_kernel_rules.txt Sorry for confusion, it is not for stable@, it was cc'ed by mistake. Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On 10/27, Ben Hutchings wrote: On Fri, 2012-10-26 at 19:46 +0200, Oleg Nesterov wrote: try_to_freeze_tasks() and cgroup_freezer rely on scheduler locks to ensure that a task doing STOPPED/TRACED - RUNNING transition can't escape freezing. This mostly works, but ptrace_stop() does not necessarily call schedule(), it can change task-state back to RUNNING and check freezing() without any lock/barrier in between. We could add the necessary barrier, but this patch changes ptrace_stop() and do_signal_stop() to use freezable_schedule(). This fixes the race, freezer_count() and freezer_should_skip() carefully avoid the race. And this simplifies the code, try_to_freeze_tasks/update_if_frozen no longer need to use task_is_stopped_or_traced() checks with the non trivial assumptions. We can rely on the mechanism which was specially designed to mark the sleeping task as frozen enough. v2: As Tejun pointed out, we can also change get_signal_to_deliver() and move try_to_freeze() up before 'relock' label. Signed-off-by: Oleg Nesterov o...@redhat.com [...] This is not the correct way to submit a change to stable. Please see Documentation/stable_kernel_rules.txt Sorry for confusion, it is not for stable@, it was cc'ed by mistake. Oleg. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On Friday, October 26, 2012 02:29:09 PM Tejun Heo wrote: > Hello, > > On Fri, Oct 26, 2012 at 11:29:56PM +0200, Rafael J. Wysocki wrote: > > Actually, what tree is it supposed to apply to? > > > > The change in kernel/cgroup_freezer.c doesn't look like anything in > > the current Linus' tree to me. > > Ooh, right. This depends on the earlier cgroup_freezer changes. > Sorry about the confusion. I'll apply it to the following branch (the > same one used for the previous cgroup_freezer updates). > > git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git cgroup-freezer OK I haven't merged it yet, so I'll get this fix along with the rest. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On Fri, 2012-10-26 at 19:46 +0200, Oleg Nesterov wrote: > try_to_freeze_tasks() and cgroup_freezer rely on scheduler locks > to ensure that a task doing STOPPED/TRACED -> RUNNING transition > can't escape freezing. This mostly works, but ptrace_stop() does > not necessarily call schedule(), it can change task->state back to > RUNNING and check freezing() without any lock/barrier in between. > > We could add the necessary barrier, but this patch changes > ptrace_stop() and do_signal_stop() to use freezable_schedule(). > This fixes the race, freezer_count() and freezer_should_skip() > carefully avoid the race. > > And this simplifies the code, try_to_freeze_tasks/update_if_frozen > no longer need to use task_is_stopped_or_traced() checks with the > non trivial assumptions. We can rely on the mechanism which was > specially designed to mark the sleeping task as "frozen enough". > > v2: As Tejun pointed out, we can also change get_signal_to_deliver() > and move try_to_freeze() up before 'relock' label. > > Signed-off-by: Oleg Nesterov [...] This is not the correct way to submit a change to stable. Please see Documentation/stable_kernel_rules.txt Ben. -- Ben Hutchings Never attribute to conspiracy what can adequately be explained by stupidity. signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On Fri, 2012-10-26 at 19:46 +0200, Oleg Nesterov wrote: try_to_freeze_tasks() and cgroup_freezer rely on scheduler locks to ensure that a task doing STOPPED/TRACED - RUNNING transition can't escape freezing. This mostly works, but ptrace_stop() does not necessarily call schedule(), it can change task-state back to RUNNING and check freezing() without any lock/barrier in between. We could add the necessary barrier, but this patch changes ptrace_stop() and do_signal_stop() to use freezable_schedule(). This fixes the race, freezer_count() and freezer_should_skip() carefully avoid the race. And this simplifies the code, try_to_freeze_tasks/update_if_frozen no longer need to use task_is_stopped_or_traced() checks with the non trivial assumptions. We can rely on the mechanism which was specially designed to mark the sleeping task as frozen enough. v2: As Tejun pointed out, we can also change get_signal_to_deliver() and move try_to_freeze() up before 'relock' label. Signed-off-by: Oleg Nesterov o...@redhat.com [...] This is not the correct way to submit a change to stable. Please see Documentation/stable_kernel_rules.txt Ben. -- Ben Hutchings Never attribute to conspiracy what can adequately be explained by stupidity. signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On Friday, October 26, 2012 02:29:09 PM Tejun Heo wrote: Hello, On Fri, Oct 26, 2012 at 11:29:56PM +0200, Rafael J. Wysocki wrote: Actually, what tree is it supposed to apply to? The change in kernel/cgroup_freezer.c doesn't look like anything in the current Linus' tree to me. Ooh, right. This depends on the earlier cgroup_freezer changes. Sorry about the confusion. I'll apply it to the following branch (the same one used for the previous cgroup_freezer updates). git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git cgroup-freezer OK I haven't merged it yet, so I'll get this fix along with the rest. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
Hello, On Fri, Oct 26, 2012 at 11:29:56PM +0200, Rafael J. Wysocki wrote: > Actually, what tree is it supposed to apply to? > > The change in kernel/cgroup_freezer.c doesn't look like anything in > the current Linus' tree to me. Ooh, right. This depends on the earlier cgroup_freezer changes. Sorry about the confusion. I'll apply it to the following branch (the same one used for the previous cgroup_freezer updates). git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git cgroup-freezer Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On Friday, October 26, 2012 11:14:17 PM Rafael J. Wysocki wrote: > On Friday, October 26, 2012 08:01:49 PM Oleg Nesterov wrote: > > On 10/26, Tejun Heo wrote: > > > > > > Acked-by: Tejun Heo > > > > Thanks! > > > > > Rafael, sorry that this one doesn't have pm cc'd > > > > Ah, sorry Rafael. Yes, I have read you email, and I was going to > > add linux-pm but forgot. > > > > > but can you please > > > pick up this one too? > > > > Please, and thanks. > > OK, but that will go to Linus in the next batch. Actually, what tree is it supposed to apply to? The change in kernel/cgroup_freezer.c doesn't look like anything in the current Linus' tree to me. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On Friday, October 26, 2012 08:01:49 PM Oleg Nesterov wrote: > On 10/26, Tejun Heo wrote: > > > > Acked-by: Tejun Heo > > Thanks! > > > Rafael, sorry that this one doesn't have pm cc'd > > Ah, sorry Rafael. Yes, I have read you email, and I was going to > add linux-pm but forgot. > > > but can you please > > pick up this one too? > > Please, and thanks. OK, but that will go to Linus in the next batch. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On 10/26, Tejun Heo wrote: > > Acked-by: Tejun Heo Thanks! > Rafael, sorry that this one doesn't have pm cc'd Ah, sorry Rafael. Yes, I have read you email, and I was going to add linux-pm but forgot. > but can you please > pick up this one too? Please, and thanks. Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On Fri, Oct 26, 2012 at 07:46:06PM +0200, Oleg Nesterov wrote: > try_to_freeze_tasks() and cgroup_freezer rely on scheduler locks > to ensure that a task doing STOPPED/TRACED -> RUNNING transition > can't escape freezing. This mostly works, but ptrace_stop() does > not necessarily call schedule(), it can change task->state back to > RUNNING and check freezing() without any lock/barrier in between. > > We could add the necessary barrier, but this patch changes > ptrace_stop() and do_signal_stop() to use freezable_schedule(). > This fixes the race, freezer_count() and freezer_should_skip() > carefully avoid the race. > > And this simplifies the code, try_to_freeze_tasks/update_if_frozen > no longer need to use task_is_stopped_or_traced() checks with the > non trivial assumptions. We can rely on the mechanism which was > specially designed to mark the sleeping task as "frozen enough". > > v2: As Tejun pointed out, we can also change get_signal_to_deliver() > and move try_to_freeze() up before 'relock' label. > > Signed-off-by: Oleg Nesterov Looks good to me. :) Acked-by: Tejun Heo Rafael, sorry that this one doesn't have pm cc'd but can you please pick up this one too? Thanks a lot. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On Fri, Oct 26, 2012 at 07:46:06PM +0200, Oleg Nesterov wrote: try_to_freeze_tasks() and cgroup_freezer rely on scheduler locks to ensure that a task doing STOPPED/TRACED - RUNNING transition can't escape freezing. This mostly works, but ptrace_stop() does not necessarily call schedule(), it can change task-state back to RUNNING and check freezing() without any lock/barrier in between. We could add the necessary barrier, but this patch changes ptrace_stop() and do_signal_stop() to use freezable_schedule(). This fixes the race, freezer_count() and freezer_should_skip() carefully avoid the race. And this simplifies the code, try_to_freeze_tasks/update_if_frozen no longer need to use task_is_stopped_or_traced() checks with the non trivial assumptions. We can rely on the mechanism which was specially designed to mark the sleeping task as frozen enough. v2: As Tejun pointed out, we can also change get_signal_to_deliver() and move try_to_freeze() up before 'relock' label. Signed-off-by: Oleg Nesterov o...@redhat.com Looks good to me. :) Acked-by: Tejun Heo t...@kernel.org Rafael, sorry that this one doesn't have pm cc'd but can you please pick up this one too? Thanks a lot. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On 10/26, Tejun Heo wrote: Acked-by: Tejun Heo t...@kernel.org Thanks! Rafael, sorry that this one doesn't have pm cc'd Ah, sorry Rafael. Yes, I have read you email, and I was going to add linux-pm but forgot. but can you please pick up this one too? Please, and thanks. Oleg. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On Friday, October 26, 2012 08:01:49 PM Oleg Nesterov wrote: On 10/26, Tejun Heo wrote: Acked-by: Tejun Heo t...@kernel.org Thanks! Rafael, sorry that this one doesn't have pm cc'd Ah, sorry Rafael. Yes, I have read you email, and I was going to add linux-pm but forgot. but can you please pick up this one too? Please, and thanks. OK, but that will go to Linus in the next batch. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
On Friday, October 26, 2012 11:14:17 PM Rafael J. Wysocki wrote: On Friday, October 26, 2012 08:01:49 PM Oleg Nesterov wrote: On 10/26, Tejun Heo wrote: Acked-by: Tejun Heo t...@kernel.org Thanks! Rafael, sorry that this one doesn't have pm cc'd Ah, sorry Rafael. Yes, I have read you email, and I was going to add linux-pm but forgot. but can you please pick up this one too? Please, and thanks. OK, but that will go to Linus in the next batch. Actually, what tree is it supposed to apply to? The change in kernel/cgroup_freezer.c doesn't look like anything in the current Linus' tree to me. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule()
Hello, On Fri, Oct 26, 2012 at 11:29:56PM +0200, Rafael J. Wysocki wrote: Actually, what tree is it supposed to apply to? The change in kernel/cgroup_freezer.c doesn't look like anything in the current Linus' tree to me. Ooh, right. This depends on the earlier cgroup_freezer changes. Sorry about the confusion. I'll apply it to the following branch (the same one used for the previous cgroup_freezer updates). git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git cgroup-freezer Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/