Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
Hi! > Rafael, > On Sat, Feb 17, 2007 at 12:24:45PM +0100, Rafael J. Wysocki wrote: > > > > Pavel, do you think we can remove the PF_NOFREEZE from bluetooth, BTW? > > The create_workqueue by default marks the worker_threads to be > non_freezable. For cpu hotplug, all workqueues can be frozen > except the "kthread" workqueue (which is single threaded, so won't > be frozen anyway). > > And a quick cscope scan shows that only the "xfslogd" and "xfsdatad" > are the only freezable workqueues. Any particular reason > for not marking rest of the non-single_threaded workqueues freezeable ?? As I said, go ahead. bluetooth has absolutely no business running while we are writing suspend image to disk. (First person suggesting suspend-to-file-on-nfs-filesystem-mounted-over-GPRS-line-connected-over-bluetooth will be punished by only getting bread and water till he implements it). Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
Rafael, On Sat, Feb 17, 2007 at 12:24:45PM +0100, Rafael J. Wysocki wrote: > > Pavel, do you think we can remove the PF_NOFREEZE from bluetooth, BTW? The create_workqueue by default marks the worker_threads to be non_freezable. For cpu hotplug, all workqueues can be frozen except the "kthread" workqueue (which is single threaded, so won't be frozen anyway). And a quick cscope scan shows that only the "xfslogd" and "xfsdatad" are the only freezable workqueues. Any particular reason for not marking rest of the non-single_threaded workqueues freezeable ?? thanks and regards gautham -- Gautham R Shenoy Linux Technology Center IBM India. "Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless!" - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
Rafael, On Sat, Feb 17, 2007 at 12:24:45PM +0100, Rafael J. Wysocki wrote: Pavel, do you think we can remove the PF_NOFREEZE from bluetooth, BTW? The create_workqueue by default marks the worker_threads to be non_freezable. For cpu hotplug, all workqueues can be frozen except the kthread workqueue (which is single threaded, so won't be frozen anyway). And a quick cscope scan shows that only the xfslogd and xfsdatad are the only freezable workqueues. Any particular reason for not marking rest of the non-single_threaded workqueues freezeable ?? thanks and regards gautham -- Gautham R Shenoy Linux Technology Center IBM India. Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless! - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
Hi! Rafael, On Sat, Feb 17, 2007 at 12:24:45PM +0100, Rafael J. Wysocki wrote: Pavel, do you think we can remove the PF_NOFREEZE from bluetooth, BTW? The create_workqueue by default marks the worker_threads to be non_freezable. For cpu hotplug, all workqueues can be frozen except the kthread workqueue (which is single threaded, so won't be frozen anyway). And a quick cscope scan shows that only the xfslogd and xfsdatad are the only freezable workqueues. Any particular reason for not marking rest of the non-single_threaded workqueues freezeable ?? As I said, go ahead. bluetooth has absolutely no business running while we are writing suspend image to disk. (First person suggesting suspend-to-file-on-nfs-filesystem-mounted-over-GPRS-line-connected-over-bluetooth will be punished by only getting bread and water till he implements it). Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Sunday, 18 February 2007 17:11, Oleg Nesterov wrote: > On 02/18, Rafael J. Wysocki wrote: > > > > > On Sunday, 18 February 2007 12:31, Oleg Nesterov wrote: > > > > > > > > A very vague idea: what if parent will do > > > > > > > > current->flags |= > > > > PF_PLEASE_CONSIDER_ME_AS_FROZEN_BUT_SET_TIF_FREEZE > > > > wait_for_completion(); > > > > try_to_freeze(); > > > > > > > > ? > > > > Hm, what about the following patch instead? > > > > The problem is that if the child enters the refrigeratior, we can't freeze > > the > > parent, because it's uninterruptible, but the child knows the parent will be > > uninterruptible until it exits, so the child can mark the parent as frozen. > > > > --- linux-2.6.20-mm2.orig/kernel/power/process.c2007-02-18 > > 15:43:30.0 +0100 > > +++ linux-2.6.20-mm2/kernel/power/process.c 2007-02-18 16:09:53.0 > > +0100 > > @@ -39,6 +39,13 @@ void refrigerator(void) > > /* Hmm, should we be allowed to suspend when there are realtime > >processes around? */ > > long save; > > + > > + /* The parent is uninterruptible and will stay so until this task exits, > > +* so we can mark it as frozen. > > +*/ > > + if (current->vfork_done) > > + frozen_process(current->parent); > > This is not safe. task->flags is not atomic, we can change ->flags only > if we know the task won't touch it itself (ptrace, thaw_process). > The parent could be interrupted, irq may play with current->flags (slab, > for example). > > Please note that ->parent may do things like ptrace_notify() before > it actually sleeps on ->vfork_done. This means that even if we could > set PF_FROZEN in a safe manner, this doesn't look like a good idea. > > > + > > + if (current->vfork_done && frozen(current->parent)) > > + current->parent->flags &= ~PF_FROZEN; > > } > > Why? If the code above works, we shouldn't take care about frozen > ->parent? I've added this for symmetry. thaw_tasks() should reset PF_FROZEN for it anyway. Okay, so I'll post the patch that implements your idea in the other thread. Greetings, Rafael - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On 02/18, Rafael J. Wysocki wrote: > > > On Sunday, 18 February 2007 12:31, Oleg Nesterov wrote: > > > > > > A very vague idea: what if parent will do > > > > > > current->flags |= PF_PLEASE_CONSIDER_ME_AS_FROZEN_BUT_SET_TIF_FREEZE > > > wait_for_completion(); > > > try_to_freeze(); > > > > > > ? > > Hm, what about the following patch instead? > > The problem is that if the child enters the refrigeratior, we can't freeze the > parent, because it's uninterruptible, but the child knows the parent will be > uninterruptible until it exits, so the child can mark the parent as frozen. > > --- linux-2.6.20-mm2.orig/kernel/power/process.c 2007-02-18 > 15:43:30.0 +0100 > +++ linux-2.6.20-mm2/kernel/power/process.c 2007-02-18 16:09:53.0 > +0100 > @@ -39,6 +39,13 @@ void refrigerator(void) > /* Hmm, should we be allowed to suspend when there are realtime > processes around? */ > long save; > + > + /* The parent is uninterruptible and will stay so until this task exits, > + * so we can mark it as frozen. > + */ > + if (current->vfork_done) > + frozen_process(current->parent); This is not safe. task->flags is not atomic, we can change ->flags only if we know the task won't touch it itself (ptrace, thaw_process). The parent could be interrupted, irq may play with current->flags (slab, for example). Please note that ->parent may do things like ptrace_notify() before it actually sleeps on ->vfork_done. This means that even if we could set PF_FROZEN in a safe manner, this doesn't look like a good idea. > + > + if (current->vfork_done && frozen(current->parent)) > + current->parent->flags &= ~PF_FROZEN; > } Why? If the code above works, we shouldn't take care about frozen ->parent? 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Sunday, 18 February 2007 13:14, Rafael J. Wysocki wrote:
> On Sunday, 18 February 2007 12:31, Oleg Nesterov wrote:
> > On 02/18, Rafael J. Wysocki wrote:
> > >
> > > On Sunday, 18 February 2007 00:47, Oleg Nesterov wrote:
> > > >
> > > > However, this means that sys_vfork() makes impossible to freeze
> > > > processes
> > > > until child exits/execs. Not good.
> > >
> > > Yes, but this also is the current behavior.
> >
> > Yes, yes, I see.
> >
> > I forgot to say that we have another problem: coredumping.
> >
> > A thread which does do_coredump() send SIGKILL to ->mm users, and sleeps
> > on ->mm->core_startup_done. Now it can't be frozen if sub-thread goes to
> > refrigerator. I think this could be solved easily if we add a check to
> > refrigerator() as you suggested for ->vfork_donw.
> >
> > > I think the real solution would be to use an interruptible completion in
> > > the
> > > vfork code. It was discussed some time ago and, IIRC, Ingo had an
> > > experimental
> > > patch that implemented it. Still, for the suspend this really is not an
> > > issue
> > > in practice, so it wasn't merged.
> >
> > It is not (afaics) so trivial to do rightly, and with this change the parent
> > will be seen as TASK_INTERRUPTIBLE even without freezer in progress.
> >
> > A very vague idea: what if parent will do
> >
> > current->flags |= PF_PLEASE_CONSIDER_ME_AS_FROZEN_BUT_SET_TIF_FREEZE
> > wait_for_completion();
> > try_to_freeze();
> >
> > ?
>
> This should work, but we'll need a separate process flag for it. If that's
> acceptable, I'd call it PF_VFORK_PARENT
Hm, what about the following patch instead?
The problem is that if the child enters the refrigeratior, we can't freeze the
parent, because it's uninterruptible, but the child knows the parent will be
uninterruptible until it exits, so the child can mark the parent as frozen.
kernel/power/process.c | 27 +--
1 file changed, 13 insertions(+), 14 deletions(-)
Index: linux-2.6.20-mm2/kernel/power/process.c
===
--- linux-2.6.20-mm2.orig/kernel/power/process.c2007-02-18
15:43:30.0 +0100
+++ linux-2.6.20-mm2/kernel/power/process.c 2007-02-18 16:09:53.0
+0100
@@ -39,6 +39,13 @@ void refrigerator(void)
/* Hmm, should we be allowed to suspend when there are realtime
processes around? */
long save;
+
+ /* The parent is uninterruptible and will stay so until this task exits,
+* so we can mark it as frozen.
+*/
+ if (current->vfork_done)
+ frozen_process(current->parent);
+
save = current->state;
pr_debug("%s entered refrigerator\n", current->comm);
@@ -53,6 +60,9 @@ void refrigerator(void)
}
pr_debug("%s left refrigerator\n", current->comm);
current->state = save;
+
+ if (current->vfork_done && frozen(current->parent))
+ current->parent->flags &= ~PF_FROZEN;
}
static inline void freeze_process(struct task_struct *p)
@@ -117,21 +127,10 @@ static unsigned int try_to_freeze_tasks(
cancel_freezing(p);
continue;
}
- if (is_user_space(p)) {
- if (!freeze_user_space)
- continue;
-
- /* Freeze the task unless there is a vfork
-* completion pending
-*/
- if (!p->vfork_done)
- freeze_process(p);
- } else {
- if (freeze_user_space)
- continue;
+ if (is_user_space(p) == !freeze_user_space)
+ continue;
- freeze_process(p);
- }
+ freeze_process(p);
todo++;
} while_each_thread(g, p);
read_unlock(_lock);
-
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On 02/18, Rafael J. Wysocki wrote:
>
> On Sunday, 18 February 2007 12:32, Oleg Nesterov wrote:
> > On 02/18, Rafael J. Wysocki wrote:
> > >
> > > On Sunday, 18 February 2007 00:42, Oleg Nesterov wrote:
> > > > On 02/17, Rafael J. Wysocki wrote:
> > > > >
> > > > > On Saturday, 17 February 2007 22:34, Oleg Nesterov wrote:
> > > > > >
> > > > > > static inline int is_user_space(struct task_struct *p)
> > > > > > {
> > > > > > return p->mm && !(p->flags & PF_BORROWED_MM);
> > > > > > }
> > > > > >
> > > > > > This doesn't look right. First, an exiting task has ->mm == NULL
> > > > > > after
> > > > > > do_exit()->exit_mm(). Probably not a problem. However,
> > > > > > PF_BORROWED_MM
> > > > > > check is racy without task_lock(), so we can have a false positive
> > > > > > as
> > > > > > well. Is it ok? We can freeze aio_wq prematurely.
> > > > >
> > > > > Right now aio_wq is not freezeable (PF_NOFREEZE).
> > > >
> > > > Right now yes, but we are going to change this?
> > >
> > > Well, is there any more reliable (and not racy) method of differentiating
> > > between kernel threads and user space processes?
> >
> > Not that I know of. At least, we can take task_lock() to really rule out
> > kernel threads at FREEZER_USER_SPACE stage.
>
> Something like this?
I think yes, as a first step.
In the long term, I think it would be really nice to have CLONE_KERNEL_THREAD
(filtered out in sys_clone). This also allows us to cleanup copy_process().
For example, we can then introduce CLONE_UNHASHED (currently denoted as pid==0)
and kill the ugly "int pid" copy_process's parameter.
Oleg.
> ---
> kernel/power/process.c |8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.20-mm2/kernel/power/process.c
> ===
> --- linux-2.6.20-mm2.orig/kernel/power/process.c
> +++ linux-2.6.20-mm2/kernel/power/process.c
> @@ -8,6 +8,7 @@
>
> #undef DEBUG
>
> +#include
> #include
> #include
> #include
> @@ -92,7 +93,12 @@ static void cancel_freezing(struct task_
>
> static inline int is_user_space(struct task_struct *p)
> {
> - return p->mm && !(p->flags & PF_BORROWED_MM);
> + int ret;
> +
> + task_lock(p);
> + ret = p->mm && !(p->flags & PF_BORROWED_MM);
> + task_unlock(p);
> + return ret;
> }
>
> static unsigned int try_to_freeze_tasks(int freeze_user_space)
-
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
Hi! > > > So I think tonight I'll start adding try_to_freeze() to the kernel > > > threads that > > > set PF_NOFREEZE. > > > > cool! While you are at it, let me try to enhance the freezer api's > > to incorporate the PFE_* flags. > > Here's a patch that adds try_to_freeze() to all kernel threads that didn't > call > it before. It shouldn't change the behavior of the threads in question, since > they won't be frozen because the are flagged as PF_NOFREEZE (of course > we are going to change this later). Looks ok. > Compile-tested on x86_64 with allmodconfig. > > Pavel, do you think we can remove the PF_NOFREEZE from bluetooth, > BTW? Yes... bluetooth has no reason to play with NOFREEZE. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Sunday, 18 February 2007 12:32, Oleg Nesterov wrote:
> On 02/18, Rafael J. Wysocki wrote:
> >
> > On Sunday, 18 February 2007 00:42, Oleg Nesterov wrote:
> > > On 02/17, Rafael J. Wysocki wrote:
> > > >
> > > > On Saturday, 17 February 2007 22:34, Oleg Nesterov wrote:
> > > > >
> > > > > static inline int is_user_space(struct task_struct *p)
> > > > > {
> > > > > return p->mm && !(p->flags & PF_BORROWED_MM);
> > > > > }
> > > > >
> > > > > This doesn't look right. First, an exiting task has ->mm == NULL after
> > > > > do_exit()->exit_mm(). Probably not a problem. However, PF_BORROWED_MM
> > > > > check is racy without task_lock(), so we can have a false positive as
> > > > > well. Is it ok? We can freeze aio_wq prematurely.
> > > >
> > > > Right now aio_wq is not freezeable (PF_NOFREEZE).
> > >
> > > Right now yes, but we are going to change this?
> >
> > Well, is there any more reliable (and not racy) method of differentiating
> > between kernel threads and user space processes?
>
> Not that I know of. At least, we can take task_lock() to really rule out
> kernel threads at FREEZER_USER_SPACE stage.
Something like this?
---
kernel/power/process.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
Index: linux-2.6.20-mm2/kernel/power/process.c
===
--- linux-2.6.20-mm2.orig/kernel/power/process.c
+++ linux-2.6.20-mm2/kernel/power/process.c
@@ -8,6 +8,7 @@
#undef DEBUG
+#include
#include
#include
#include
@@ -92,7 +93,12 @@ static void cancel_freezing(struct task_
static inline int is_user_space(struct task_struct *p)
{
- return p->mm && !(p->flags & PF_BORROWED_MM);
+ int ret;
+
+ task_lock(p);
+ ret = p->mm && !(p->flags & PF_BORROWED_MM);
+ task_unlock(p);
+ return ret;
}
static unsigned int try_to_freeze_tasks(int freeze_user_space)
-
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Sunday, 18 February 2007 12:31, Oleg Nesterov wrote: > On 02/18, Rafael J. Wysocki wrote: > > > > On Sunday, 18 February 2007 00:47, Oleg Nesterov wrote: > > > > > > However, this means that sys_vfork() makes impossible to freeze processes > > > until child exits/execs. Not good. > > > > Yes, but this also is the current behavior. > > Yes, yes, I see. > > I forgot to say that we have another problem: coredumping. > > A thread which does do_coredump() send SIGKILL to ->mm users, and sleeps > on ->mm->core_startup_done. Now it can't be frozen if sub-thread goes to > refrigerator. I think this could be solved easily if we add a check to > refrigerator() as you suggested for ->vfork_donw. > > > I think the real solution would be to use an interruptible completion in the > > vfork code. It was discussed some time ago and, IIRC, Ingo had an > > experimental > > patch that implemented it. Still, for the suspend this really is not an > > issue > > in practice, so it wasn't merged. > > It is not (afaics) so trivial to do rightly, and with this change the parent > will be seen as TASK_INTERRUPTIBLE even without freezer in progress. > > A very vague idea: what if parent will do > > current->flags |= PF_PLEASE_CONSIDER_ME_AS_FROZEN_BUT_SET_TIF_FREEZE > wait_for_completion(); > try_to_freeze(); > > ? This should work, but we'll need a separate process flag for it. If that's acceptable, I'd call it PF_VFORK_PARENT Greetings, Rafael - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On 02/18, Rafael J. Wysocki wrote:
>
> On Sunday, 18 February 2007 00:42, Oleg Nesterov wrote:
> > On 02/17, Rafael J. Wysocki wrote:
> > >
> > > On Saturday, 17 February 2007 22:34, Oleg Nesterov wrote:
> > > >
> > > > static inline int is_user_space(struct task_struct *p)
> > > > {
> > > > return p->mm && !(p->flags & PF_BORROWED_MM);
> > > > }
> > > >
> > > > This doesn't look right. First, an exiting task has ->mm == NULL after
> > > > do_exit()->exit_mm(). Probably not a problem. However, PF_BORROWED_MM
> > > > check is racy without task_lock(), so we can have a false positive as
> > > > well. Is it ok? We can freeze aio_wq prematurely.
> > >
> > > Right now aio_wq is not freezeable (PF_NOFREEZE).
> >
> > Right now yes, but we are going to change this?
>
> Well, is there any more reliable (and not racy) method of differentiating
> between kernel threads and user space processes?
Not that I know of. At least, we can take task_lock() to really rule out
kernel threads at FREEZER_USER_SPACE stage.
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On 02/18, Rafael J. Wysocki wrote: > > On Sunday, 18 February 2007 00:47, Oleg Nesterov wrote: > > > > However, this means that sys_vfork() makes impossible to freeze processes > > until child exits/execs. Not good. > > Yes, but this also is the current behavior. Yes, yes, I see. I forgot to say that we have another problem: coredumping. A thread which does do_coredump() send SIGKILL to ->mm users, and sleeps on ->mm->core_startup_done. Now it can't be frozen if sub-thread goes to refrigerator. I think this could be solved easily if we add a check to refrigerator() as you suggested for ->vfork_donw. > I think the real solution would be to use an interruptible completion in the > vfork code. It was discussed some time ago and, IIRC, Ingo had an > experimental > patch that implemented it. Still, for the suspend this really is not an issue > in practice, so it wasn't merged. It is not (afaics) so trivial to do rightly, and with this change the parent will be seen as TASK_INTERRUPTIBLE even without freezer in progress. A very vague idea: what if parent will do current->flags |= PF_PLEASE_CONSIDER_ME_AS_FROZEN_BUT_SET_TIF_FREEZE wait_for_completion(); try_to_freeze(); ? > It may be a good time to solve this problem now. :-) Yes, I think so :) 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Sunday, 18 February 2007 00:42, Oleg Nesterov wrote:
> On 02/17, Rafael J. Wysocki wrote:
> >
> > On Saturday, 17 February 2007 22:34, Oleg Nesterov wrote:
> > >
> > > static inline int is_user_space(struct task_struct *p)
> > > {
> > > return p->mm && !(p->flags & PF_BORROWED_MM);
> > > }
> > >
> > > This doesn't look right. First, an exiting task has ->mm == NULL after
> > > do_exit()->exit_mm(). Probably not a problem. However, PF_BORROWED_MM
> > > check is racy without task_lock(), so we can have a false positive as
> > > well. Is it ok? We can freeze aio_wq prematurely.
> >
> > Right now aio_wq is not freezeable (PF_NOFREEZE).
>
> Right now yes, but we are going to change this?
Well, is there any more reliable (and not racy) method of differentiating
between kernel threads and user space processes?
> > > cancel_freezing(p);
> > > continue;
> > >
> > > Is it right? Shouldn't we increment "todo" counter?
> >
> > No. It would be wrong to do that, because TASK_TRACED tasks with frozen
> > parents cannot be frozen any further.
>
> TASK_TRACED task could be woken by SIGKILL. cancel_freezing() clears
> TIF_FREEZE.
> The task may start do_exit() when try_to_freeze_tasks() returns "success".
> Probably not a problem.
Yup.
Greetings,
Rafael
-
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Sunday, 18 February 2007 00:47, Oleg Nesterov wrote: > On 02/18, Oleg Nesterov wrote: > > > > On 02/17, Rafael J. Wysocki wrote: > > > > > > Alternatively, we can move the check into refrigerator(), like this: > > > > > > --- linux-2.6.20-git13.orig/kernel/power/process.c > > > +++ linux-2.6.20-git13/kernel/power/process.c > > > @@ -39,6 +39,11 @@ void refrigerator(void) > > > /* Hmm, should we be allowed to suspend when there are realtime > > > processes around? */ > > > long save; > > > + > > > + /* Freeze the task unless there is a vfork completion pending */ > > > + if (current->vfork_done) > > > + return; > > > + > > > > This means that "current" returns to user space (get_signal_to_deliver > > will clear TIF_SIGPENDING) and runs. While try_to_freeze_tasks() thinks > > it is frozen. > > Ah, sorry. I am wrong, current has no PF_FROZEN yet. > > However, this means that sys_vfork() makes impossible to freeze processes > until child exits/execs. Not good. Yes, but this also is the current behavior. I think the real solution would be to use an interruptible completion in the vfork code. It was discussed some time ago and, IIRC, Ingo had an experimental patch that implemented it. Still, for the suspend this really is not an issue in practice, so it wasn't merged. It may be a good time to solve this problem now. :-) Greetings, Rafael - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Sunday, 18 February 2007 00:42, Oleg Nesterov wrote:
On 02/17, Rafael J. Wysocki wrote:
On Saturday, 17 February 2007 22:34, Oleg Nesterov wrote:
static inline int is_user_space(struct task_struct *p)
{
return p-mm !(p-flags PF_BORROWED_MM);
}
This doesn't look right. First, an exiting task has -mm == NULL after
do_exit()-exit_mm(). Probably not a problem. However, PF_BORROWED_MM
check is racy without task_lock(), so we can have a false positive as
well. Is it ok? We can freeze aio_wq prematurely.
Right now aio_wq is not freezeable (PF_NOFREEZE).
Right now yes, but we are going to change this?
Well, is there any more reliable (and not racy) method of differentiating
between kernel threads and user space processes?
cancel_freezing(p);
continue;
Is it right? Shouldn't we increment todo counter?
No. It would be wrong to do that, because TASK_TRACED tasks with frozen
parents cannot be frozen any further.
TASK_TRACED task could be woken by SIGKILL. cancel_freezing() clears
TIF_FREEZE.
The task may start do_exit() when try_to_freeze_tasks() returns success.
Probably not a problem.
Yup.
Greetings,
Rafael
-
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Sunday, 18 February 2007 00:47, Oleg Nesterov wrote: On 02/18, Oleg Nesterov wrote: On 02/17, Rafael J. Wysocki wrote: Alternatively, we can move the check into refrigerator(), like this: --- linux-2.6.20-git13.orig/kernel/power/process.c +++ linux-2.6.20-git13/kernel/power/process.c @@ -39,6 +39,11 @@ void refrigerator(void) /* Hmm, should we be allowed to suspend when there are realtime processes around? */ long save; + + /* Freeze the task unless there is a vfork completion pending */ + if (current-vfork_done) + return; + This means that current returns to user space (get_signal_to_deliver will clear TIF_SIGPENDING) and runs. While try_to_freeze_tasks() thinks it is frozen. Ah, sorry. I am wrong, current has no PF_FROZEN yet. However, this means that sys_vfork() makes impossible to freeze processes until child exits/execs. Not good. Yes, but this also is the current behavior. I think the real solution would be to use an interruptible completion in the vfork code. It was discussed some time ago and, IIRC, Ingo had an experimental patch that implemented it. Still, for the suspend this really is not an issue in practice, so it wasn't merged. It may be a good time to solve this problem now. :-) Greetings, Rafael - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On 02/18, Rafael J. Wysocki wrote: On Sunday, 18 February 2007 00:47, Oleg Nesterov wrote: However, this means that sys_vfork() makes impossible to freeze processes until child exits/execs. Not good. Yes, but this also is the current behavior. Yes, yes, I see. I forgot to say that we have another problem: coredumping. A thread which does do_coredump() send SIGKILL to -mm users, and sleeps on -mm-core_startup_done. Now it can't be frozen if sub-thread goes to refrigerator. I think this could be solved easily if we add a check to refrigerator() as you suggested for -vfork_donw. I think the real solution would be to use an interruptible completion in the vfork code. It was discussed some time ago and, IIRC, Ingo had an experimental patch that implemented it. Still, for the suspend this really is not an issue in practice, so it wasn't merged. It is not (afaics) so trivial to do rightly, and with this change the parent will be seen as TASK_INTERRUPTIBLE even without freezer in progress. A very vague idea: what if parent will do current-flags |= PF_PLEASE_CONSIDER_ME_AS_FROZEN_BUT_SET_TIF_FREEZE wait_for_completion(vfork); try_to_freeze(); ? It may be a good time to solve this problem now. :-) Yes, I think so :) 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On 02/18, Rafael J. Wysocki wrote:
On Sunday, 18 February 2007 00:42, Oleg Nesterov wrote:
On 02/17, Rafael J. Wysocki wrote:
On Saturday, 17 February 2007 22:34, Oleg Nesterov wrote:
static inline int is_user_space(struct task_struct *p)
{
return p-mm !(p-flags PF_BORROWED_MM);
}
This doesn't look right. First, an exiting task has -mm == NULL after
do_exit()-exit_mm(). Probably not a problem. However, PF_BORROWED_MM
check is racy without task_lock(), so we can have a false positive as
well. Is it ok? We can freeze aio_wq prematurely.
Right now aio_wq is not freezeable (PF_NOFREEZE).
Right now yes, but we are going to change this?
Well, is there any more reliable (and not racy) method of differentiating
between kernel threads and user space processes?
Not that I know of. At least, we can take task_lock() to really rule out
kernel threads at FREEZER_USER_SPACE stage.
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Sunday, 18 February 2007 12:31, Oleg Nesterov wrote: On 02/18, Rafael J. Wysocki wrote: On Sunday, 18 February 2007 00:47, Oleg Nesterov wrote: However, this means that sys_vfork() makes impossible to freeze processes until child exits/execs. Not good. Yes, but this also is the current behavior. Yes, yes, I see. I forgot to say that we have another problem: coredumping. A thread which does do_coredump() send SIGKILL to -mm users, and sleeps on -mm-core_startup_done. Now it can't be frozen if sub-thread goes to refrigerator. I think this could be solved easily if we add a check to refrigerator() as you suggested for -vfork_donw. I think the real solution would be to use an interruptible completion in the vfork code. It was discussed some time ago and, IIRC, Ingo had an experimental patch that implemented it. Still, for the suspend this really is not an issue in practice, so it wasn't merged. It is not (afaics) so trivial to do rightly, and with this change the parent will be seen as TASK_INTERRUPTIBLE even without freezer in progress. A very vague idea: what if parent will do current-flags |= PF_PLEASE_CONSIDER_ME_AS_FROZEN_BUT_SET_TIF_FREEZE wait_for_completion(vfork); try_to_freeze(); ? This should work, but we'll need a separate process flag for it. If that's acceptable, I'd call it PF_VFORK_PARENT Greetings, Rafael - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Sunday, 18 February 2007 12:32, Oleg Nesterov wrote:
On 02/18, Rafael J. Wysocki wrote:
On Sunday, 18 February 2007 00:42, Oleg Nesterov wrote:
On 02/17, Rafael J. Wysocki wrote:
On Saturday, 17 February 2007 22:34, Oleg Nesterov wrote:
static inline int is_user_space(struct task_struct *p)
{
return p-mm !(p-flags PF_BORROWED_MM);
}
This doesn't look right. First, an exiting task has -mm == NULL after
do_exit()-exit_mm(). Probably not a problem. However, PF_BORROWED_MM
check is racy without task_lock(), so we can have a false positive as
well. Is it ok? We can freeze aio_wq prematurely.
Right now aio_wq is not freezeable (PF_NOFREEZE).
Right now yes, but we are going to change this?
Well, is there any more reliable (and not racy) method of differentiating
between kernel threads and user space processes?
Not that I know of. At least, we can take task_lock() to really rule out
kernel threads at FREEZER_USER_SPACE stage.
Something like this?
---
kernel/power/process.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
Index: linux-2.6.20-mm2/kernel/power/process.c
===
--- linux-2.6.20-mm2.orig/kernel/power/process.c
+++ linux-2.6.20-mm2/kernel/power/process.c
@@ -8,6 +8,7 @@
#undef DEBUG
+#include linux/sched.h
#include linux/smp_lock.h
#include linux/interrupt.h
#include linux/suspend.h
@@ -92,7 +93,12 @@ static void cancel_freezing(struct task_
static inline int is_user_space(struct task_struct *p)
{
- return p-mm !(p-flags PF_BORROWED_MM);
+ int ret;
+
+ task_lock(p);
+ ret = p-mm !(p-flags PF_BORROWED_MM);
+ task_unlock(p);
+ return ret;
}
static unsigned int try_to_freeze_tasks(int freeze_user_space)
-
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
Hi! So I think tonight I'll start adding try_to_freeze() to the kernel threads that set PF_NOFREEZE. cool! While you are at it, let me try to enhance the freezer api's to incorporate the PFE_* flags. Here's a patch that adds try_to_freeze() to all kernel threads that didn't call it before. It shouldn't change the behavior of the threads in question, since they won't be frozen because the are flagged as PF_NOFREEZE (of course we are going to change this later). Looks ok. Compile-tested on x86_64 with allmodconfig. Pavel, do you think we can remove the PF_NOFREEZE from bluetooth, BTW? Yes... bluetooth has no reason to play with NOFREEZE. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On 02/18, Rafael J. Wysocki wrote:
On Sunday, 18 February 2007 12:32, Oleg Nesterov wrote:
On 02/18, Rafael J. Wysocki wrote:
On Sunday, 18 February 2007 00:42, Oleg Nesterov wrote:
On 02/17, Rafael J. Wysocki wrote:
On Saturday, 17 February 2007 22:34, Oleg Nesterov wrote:
static inline int is_user_space(struct task_struct *p)
{
return p-mm !(p-flags PF_BORROWED_MM);
}
This doesn't look right. First, an exiting task has -mm == NULL
after
do_exit()-exit_mm(). Probably not a problem. However,
PF_BORROWED_MM
check is racy without task_lock(), so we can have a false positive
as
well. Is it ok? We can freeze aio_wq prematurely.
Right now aio_wq is not freezeable (PF_NOFREEZE).
Right now yes, but we are going to change this?
Well, is there any more reliable (and not racy) method of differentiating
between kernel threads and user space processes?
Not that I know of. At least, we can take task_lock() to really rule out
kernel threads at FREEZER_USER_SPACE stage.
Something like this?
I think yes, as a first step.
In the long term, I think it would be really nice to have CLONE_KERNEL_THREAD
(filtered out in sys_clone). This also allows us to cleanup copy_process().
For example, we can then introduce CLONE_UNHASHED (currently denoted as pid==0)
and kill the ugly int pid copy_process's parameter.
Oleg.
---
kernel/power/process.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
Index: linux-2.6.20-mm2/kernel/power/process.c
===
--- linux-2.6.20-mm2.orig/kernel/power/process.c
+++ linux-2.6.20-mm2/kernel/power/process.c
@@ -8,6 +8,7 @@
#undef DEBUG
+#include linux/sched.h
#include linux/smp_lock.h
#include linux/interrupt.h
#include linux/suspend.h
@@ -92,7 +93,12 @@ static void cancel_freezing(struct task_
static inline int is_user_space(struct task_struct *p)
{
- return p-mm !(p-flags PF_BORROWED_MM);
+ int ret;
+
+ task_lock(p);
+ ret = p-mm !(p-flags PF_BORROWED_MM);
+ task_unlock(p);
+ return ret;
}
static unsigned int try_to_freeze_tasks(int freeze_user_space)
-
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Sunday, 18 February 2007 13:14, Rafael J. Wysocki wrote:
On Sunday, 18 February 2007 12:31, Oleg Nesterov wrote:
On 02/18, Rafael J. Wysocki wrote:
On Sunday, 18 February 2007 00:47, Oleg Nesterov wrote:
However, this means that sys_vfork() makes impossible to freeze
processes
until child exits/execs. Not good.
Yes, but this also is the current behavior.
Yes, yes, I see.
I forgot to say that we have another problem: coredumping.
A thread which does do_coredump() send SIGKILL to -mm users, and sleeps
on -mm-core_startup_done. Now it can't be frozen if sub-thread goes to
refrigerator. I think this could be solved easily if we add a check to
refrigerator() as you suggested for -vfork_donw.
I think the real solution would be to use an interruptible completion in
the
vfork code. It was discussed some time ago and, IIRC, Ingo had an
experimental
patch that implemented it. Still, for the suspend this really is not an
issue
in practice, so it wasn't merged.
It is not (afaics) so trivial to do rightly, and with this change the parent
will be seen as TASK_INTERRUPTIBLE even without freezer in progress.
A very vague idea: what if parent will do
current-flags |= PF_PLEASE_CONSIDER_ME_AS_FROZEN_BUT_SET_TIF_FREEZE
wait_for_completion(vfork);
try_to_freeze();
?
This should work, but we'll need a separate process flag for it. If that's
acceptable, I'd call it PF_VFORK_PARENT
Hm, what about the following patch instead?
The problem is that if the child enters the refrigeratior, we can't freeze the
parent, because it's uninterruptible, but the child knows the parent will be
uninterruptible until it exits, so the child can mark the parent as frozen.
kernel/power/process.c | 27 +--
1 file changed, 13 insertions(+), 14 deletions(-)
Index: linux-2.6.20-mm2/kernel/power/process.c
===
--- linux-2.6.20-mm2.orig/kernel/power/process.c2007-02-18
15:43:30.0 +0100
+++ linux-2.6.20-mm2/kernel/power/process.c 2007-02-18 16:09:53.0
+0100
@@ -39,6 +39,13 @@ void refrigerator(void)
/* Hmm, should we be allowed to suspend when there are realtime
processes around? */
long save;
+
+ /* The parent is uninterruptible and will stay so until this task exits,
+* so we can mark it as frozen.
+*/
+ if (current-vfork_done)
+ frozen_process(current-parent);
+
save = current-state;
pr_debug(%s entered refrigerator\n, current-comm);
@@ -53,6 +60,9 @@ void refrigerator(void)
}
pr_debug(%s left refrigerator\n, current-comm);
current-state = save;
+
+ if (current-vfork_done frozen(current-parent))
+ current-parent-flags = ~PF_FROZEN;
}
static inline void freeze_process(struct task_struct *p)
@@ -117,21 +127,10 @@ static unsigned int try_to_freeze_tasks(
cancel_freezing(p);
continue;
}
- if (is_user_space(p)) {
- if (!freeze_user_space)
- continue;
-
- /* Freeze the task unless there is a vfork
-* completion pending
-*/
- if (!p-vfork_done)
- freeze_process(p);
- } else {
- if (freeze_user_space)
- continue;
+ if (is_user_space(p) == !freeze_user_space)
+ continue;
- freeze_process(p);
- }
+ freeze_process(p);
todo++;
} while_each_thread(g, p);
read_unlock(tasklist_lock);
-
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On 02/18, Rafael J. Wysocki wrote: On Sunday, 18 February 2007 12:31, Oleg Nesterov wrote: A very vague idea: what if parent will do current-flags |= PF_PLEASE_CONSIDER_ME_AS_FROZEN_BUT_SET_TIF_FREEZE wait_for_completion(vfork); try_to_freeze(); ? Hm, what about the following patch instead? The problem is that if the child enters the refrigeratior, we can't freeze the parent, because it's uninterruptible, but the child knows the parent will be uninterruptible until it exits, so the child can mark the parent as frozen. --- linux-2.6.20-mm2.orig/kernel/power/process.c 2007-02-18 15:43:30.0 +0100 +++ linux-2.6.20-mm2/kernel/power/process.c 2007-02-18 16:09:53.0 +0100 @@ -39,6 +39,13 @@ void refrigerator(void) /* Hmm, should we be allowed to suspend when there are realtime processes around? */ long save; + + /* The parent is uninterruptible and will stay so until this task exits, + * so we can mark it as frozen. + */ + if (current-vfork_done) + frozen_process(current-parent); This is not safe. task-flags is not atomic, we can change -flags only if we know the task won't touch it itself (ptrace, thaw_process). The parent could be interrupted, irq may play with current-flags (slab, for example). Please note that -parent may do things like ptrace_notify() before it actually sleeps on -vfork_done. This means that even if we could set PF_FROZEN in a safe manner, this doesn't look like a good idea. + + if (current-vfork_done frozen(current-parent)) + current-parent-flags = ~PF_FROZEN; } Why? If the code above works, we shouldn't take care about frozen -parent? 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Sunday, 18 February 2007 17:11, Oleg Nesterov wrote: On 02/18, Rafael J. Wysocki wrote: On Sunday, 18 February 2007 12:31, Oleg Nesterov wrote: A very vague idea: what if parent will do current-flags |= PF_PLEASE_CONSIDER_ME_AS_FROZEN_BUT_SET_TIF_FREEZE wait_for_completion(vfork); try_to_freeze(); ? Hm, what about the following patch instead? The problem is that if the child enters the refrigeratior, we can't freeze the parent, because it's uninterruptible, but the child knows the parent will be uninterruptible until it exits, so the child can mark the parent as frozen. --- linux-2.6.20-mm2.orig/kernel/power/process.c2007-02-18 15:43:30.0 +0100 +++ linux-2.6.20-mm2/kernel/power/process.c 2007-02-18 16:09:53.0 +0100 @@ -39,6 +39,13 @@ void refrigerator(void) /* Hmm, should we be allowed to suspend when there are realtime processes around? */ long save; + + /* The parent is uninterruptible and will stay so until this task exits, +* so we can mark it as frozen. +*/ + if (current-vfork_done) + frozen_process(current-parent); This is not safe. task-flags is not atomic, we can change -flags only if we know the task won't touch it itself (ptrace, thaw_process). The parent could be interrupted, irq may play with current-flags (slab, for example). Please note that -parent may do things like ptrace_notify() before it actually sleeps on -vfork_done. This means that even if we could set PF_FROZEN in a safe manner, this doesn't look like a good idea. + + if (current-vfork_done frozen(current-parent)) + current-parent-flags = ~PF_FROZEN; } Why? If the code above works, we shouldn't take care about frozen -parent? I've added this for symmetry. thaw_tasks() should reset PF_FROZEN for it anyway. Okay, so I'll post the patch that implements your idea in the other thread. Greetings, Rafael - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On 02/18, Oleg Nesterov wrote: > > On 02/17, Rafael J. Wysocki wrote: > > > > Alternatively, we can move the check into refrigerator(), like this: > > > > --- linux-2.6.20-git13.orig/kernel/power/process.c > > +++ linux-2.6.20-git13/kernel/power/process.c > > @@ -39,6 +39,11 @@ void refrigerator(void) > > /* Hmm, should we be allowed to suspend when there are realtime > >processes around? */ > > long save; > > + > > + /* Freeze the task unless there is a vfork completion pending */ > > + if (current->vfork_done) > > + return; > > + > > This means that "current" returns to user space (get_signal_to_deliver > will clear TIF_SIGPENDING) and runs. While try_to_freeze_tasks() thinks > it is frozen. Ah, sorry. I am wrong, current has no PF_FROZEN yet. However, this means that sys_vfork() makes impossible to freeze processes until child exits/execs. Not good. 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On 02/17, Rafael J. Wysocki wrote:
>
> On Saturday, 17 February 2007 22:34, Oleg Nesterov wrote:
> >
> > static inline int is_user_space(struct task_struct *p)
> > {
> > return p->mm && !(p->flags & PF_BORROWED_MM);
> > }
> >
> > This doesn't look right. First, an exiting task has ->mm == NULL after
> > do_exit()->exit_mm(). Probably not a problem. However, PF_BORROWED_MM
> > check is racy without task_lock(), so we can have a false positive as
> > well. Is it ok? We can freeze aio_wq prematurely.
>
> Right now aio_wq is not freezeable (PF_NOFREEZE).
Right now yes, but we are going to change this?
> > cancel_freezing(p);
> > continue;
> >
> > Is it right? Shouldn't we increment "todo" counter?
>
> No. It would be wrong to do that, because TASK_TRACED tasks with frozen
> parents cannot be frozen any further.
TASK_TRACED task could be woken by SIGKILL. cancel_freezing() clears TIF_FREEZE.
The task may start do_exit() when try_to_freeze_tasks() returns "success".
Probably not a problem.
> > if (!p->vfork_done)
> > freeze_process(p);
> >
> >
> > Racy. do_fork(CLONE_VFORK) first does copy_process() which puts 'p' on
> > the task list and unlocks tasklist_lock. This means that 'p' is visible
> > to try_to_freeze_tasks(), and p->vfork_done == NULL. try_to_freeze_tasks()
> > sets TIF_FREEZE.
> >
> > Now, do_fork() continues, sets ->vfork_done, p goes to user space, notices
> > the fake signal and goes to refrigerator while its parent is blocked on
> > "struct completion vfork". Freezing failed.
>
> You are right, but this has never happened, AFAICS.
>
> > So, shouldn't we do
> >
> > if (p->vfork_done)
> > cancel_freezing(p);
> >
> > instead?
>
> I don't think so. If p hasn't got TIF_FREEZE set yet or it has already been
> frozen, cancel_freezing(p) is a noop.
Yes, I misread cancel_freezing(), it doesn't wake up the task if it is frozen.
> Alternatively, we can move the check into refrigerator(), like this:
>
> --- linux-2.6.20-git13.orig/kernel/power/process.c
> +++ linux-2.6.20-git13/kernel/power/process.c
> @@ -39,6 +39,11 @@ void refrigerator(void)
> /* Hmm, should we be allowed to suspend when there are realtime
> processes around? */
> long save;
> +
> + /* Freeze the task unless there is a vfork completion pending */
> + if (current->vfork_done)
> + return;
> +
This means that "current" returns to user space (get_signal_to_deliver
will clear TIF_SIGPENDING) and runs. While try_to_freeze_tasks() thinks
it is frozen.
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Saturday, 17 February 2007 22:34, Oleg Nesterov wrote:
> Rafael, I am trying to understand try_to_freeze_tasks(), and I have a
> couple of questions.
>
> static inline int is_user_space(struct task_struct *p)
> {
> return p->mm && !(p->flags & PF_BORROWED_MM);
> }
>
> This doesn't look right. First, an exiting task has ->mm == NULL after
> do_exit()->exit_mm(). Probably not a problem. However, PF_BORROWED_MM
> check is racy without task_lock(), so we can have a false positive as
> well. Is it ok? We can freeze aio_wq prematurely.
Right now aio_wq is not freezeable (PF_NOFREEZE).
>
>
> try_to_freeze_tasks:
>
> do_each_thread(g, p) {
>
> if (p->state == TASK_TRACED && frozen(p->parent)) {
>
> Why we are doing this check outside of "if (is_user_space(p))" ?
> Not a bug of course, but looks strange.
For no particular reason.
>
> cancel_freezing(p);
> continue;
>
> Is it right? Shouldn't we increment "todo" counter?
No. It would be wrong to do that, because TASK_TRACED tasks with frozen
parents cannot be frozen any further.
>
> }
> if (is_user_space(p)) {
> if (!freeze_user_space)
> continue;
>
> /* Freeze the task unless there is a vfork
>* completion pending
>*/
> if (!p->vfork_done)
> freeze_process(p);
>
>
> Racy. do_fork(CLONE_VFORK) first does copy_process() which puts 'p' on
> the task list and unlocks tasklist_lock. This means that 'p' is visible
> to try_to_freeze_tasks(), and p->vfork_done == NULL. try_to_freeze_tasks()
> sets TIF_FREEZE.
>
> Now, do_fork() continues, sets ->vfork_done, p goes to user space, notices
> the fake signal and goes to refrigerator while its parent is blocked on
> "struct completion vfork". Freezing failed.
You are right, but this has never happened, AFAICS.
> So, shouldn't we do
>
> if (p->vfork_done)
> cancel_freezing(p);
>
> instead?
I don't think so. If p hasn't got TIF_FREEZE set yet or it has already been
frozen, cancel_freezing(p) is a noop.
Alternatively, we can move the check into refrigerator(), like this:
---
kernel/power/process.c | 21 +++--
1 file changed, 7 insertions(+), 14 deletions(-)
Index: linux-2.6.20-git13/kernel/power/process.c
===
--- linux-2.6.20-git13.orig/kernel/power/process.c
+++ linux-2.6.20-git13/kernel/power/process.c
@@ -39,6 +39,11 @@ void refrigerator(void)
/* Hmm, should we be allowed to suspend when there are realtime
processes around? */
long save;
+
+ /* Freeze the task unless there is a vfork completion pending */
+ if (current->vfork_done)
+ return;
+
save = current->state;
pr_debug("%s entered refrigerator\n", current->comm);
@@ -112,22 +117,10 @@ static unsigned int try_to_freeze_tasks(
cancel_freezing(p);
continue;
}
- if (is_user_space(p)) {
- if (!freeze_user_space)
- continue;
-
- /* Freeze the task unless there is a vfork
-* completion pending
-*/
- if (!p->vfork_done)
- freeze_process(p);
- } else {
- if (freeze_user_space)
- continue;
-
+ if (is_user_space(p) == !!freeze_user_space) {
freeze_process(p);
+ todo++;
}
- todo++;
} while_each_thread(g, p);
read_unlock(_lock);
yield();/* Yield is okay here */
-
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
Rafael, I am trying to understand try_to_freeze_tasks(), and I have a
couple of questions.
static inline int is_user_space(struct task_struct *p)
{
return p->mm && !(p->flags & PF_BORROWED_MM);
}
This doesn't look right. First, an exiting task has ->mm == NULL after
do_exit()->exit_mm(). Probably not a problem. However, PF_BORROWED_MM
check is racy without task_lock(), so we can have a false positive as
well. Is it ok? We can freeze aio_wq prematurely.
try_to_freeze_tasks:
do_each_thread(g, p) {
if (p->state == TASK_TRACED && frozen(p->parent)) {
Why we are doing this check outside of "if (is_user_space(p))" ?
Not a bug of course, but looks strange.
cancel_freezing(p);
continue;
Is it right? Shouldn't we increment "todo" counter?
}
if (is_user_space(p)) {
if (!freeze_user_space)
continue;
/* Freeze the task unless there is a vfork
* completion pending
*/
if (!p->vfork_done)
freeze_process(p);
Racy. do_fork(CLONE_VFORK) first does copy_process() which puts 'p' on
the task list and unlocks tasklist_lock. This means that 'p' is visible
to try_to_freeze_tasks(), and p->vfork_done == NULL. try_to_freeze_tasks()
sets TIF_FREEZE.
Now, do_fork() continues, sets ->vfork_done, p goes to user space, notices
the fake signal and goes to refrigerator while its parent is blocked on
"struct completion vfork". Freezing failed.
So, shouldn't we do
if (p->vfork_done)
cancel_freezing(p);
instead?
Thanks,
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Thursday, 15 February 2007 15:25, Gautham R Shenoy wrote:
> On Thu, Feb 15, 2007 at 02:31:08PM +0100, Rafael J. Wysocki wrote:
> >
> > So I think tonight I'll start adding try_to_freeze() to the kernel threads
> > that
> > set PF_NOFREEZE.
>
> cool! While you are at it, let me try to enhance the freezer api's
> to incorporate the PFE_* flags.
Here's a patch that adds try_to_freeze() to all kernel threads that didn't call
it before. It shouldn't change the behavior of the threads in question, since
they won't be frozen because the are flagged as PF_NOFREEZE (of course
we are going to change this later).
Compile-tested on x86_64 with allmodconfig.
Pavel, do you think we can remove the PF_NOFREEZE from bluetooth, BTW?
arch/i386/kernel/apm.c |2 ++
drivers/block/loop.c|2 ++
drivers/char/apm-emulation.c|3 +++
drivers/ieee1394/ieee1394_core.c|3 +++
drivers/md/md.c |2 ++
drivers/mmc/mmc_queue.c |3 +++
drivers/mtd/mtd_blkdevs.c |3 +++
drivers/scsi/libsas/sas_scsi_host.c |3 +++
drivers/scsi/scsi_error.c |3 +++
drivers/usb/storage/usb.c |2 ++
kernel/rcutorture.c |2 ++
kernel/softirq.c|2 ++
kernel/softlockup.c |2 ++
kernel/workqueue.c |3 +--
net/bluetooth/bnep/core.c |5 -
net/bluetooth/cmtp/core.c |3 +++
net/bluetooth/hidp/core.c |3 +++
net/bluetooth/rfcomm/core.c |3 +++
18 files changed, 46 insertions(+), 3 deletions(-)
Index: linux-2.6.20-mm1/arch/i386/kernel/apm.c
===
--- linux-2.6.20-mm1.orig/arch/i386/kernel/apm.c2007-02-17
00:43:19.0 +0100
+++ linux-2.6.20-mm1/arch/i386/kernel/apm.c 2007-02-17 00:43:31.0
+0100
@@ -227,6 +227,7 @@
#include
#include
#include
+#include
#include
#include
@@ -1402,6 +1403,7 @@ static void apm_mainloop(void)
add_wait_queue(_waitqueue, );
set_current_state(TASK_INTERRUPTIBLE);
for (;;) {
+ try_to_freeze();
schedule_timeout(APM_CHECK_TIMEOUT);
if (kthread_should_stop())
break;
Index: linux-2.6.20-mm1/drivers/md/md.c
===
--- linux-2.6.20-mm1.orig/drivers/md/md.c 2007-02-17 00:43:19.0
+0100
+++ linux-2.6.20-mm1/drivers/md/md.c2007-02-17 00:43:31.0 +0100
@@ -4513,6 +4513,8 @@ static int md_thread(void * arg)
|| kthread_should_stop(),
thread->timeout);
+ try_to_freeze();
+
clear_bit(THREAD_WAKEUP, >flags);
thread->run(thread->mddev);
Index: linux-2.6.20-mm1/drivers/mmc/mmc_queue.c
===
--- linux-2.6.20-mm1.orig/drivers/mmc/mmc_queue.c 2007-02-17
00:43:19.0 +0100
+++ linux-2.6.20-mm1/drivers/mmc/mmc_queue.c2007-02-17 00:43:31.0
+0100
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
@@ -70,6 +71,8 @@ static int mmc_queue_thread(void *d)
do {
struct request *req = NULL;
+ try_to_freeze();
+
spin_lock_irq(q->queue_lock);
set_current_state(TASK_INTERRUPTIBLE);
req = elv_next_request(q);
Index: linux-2.6.20-mm1/drivers/mtd/mtd_blkdevs.c
===
--- linux-2.6.20-mm1.orig/drivers/mtd/mtd_blkdevs.c 2007-02-17
00:43:19.0 +0100
+++ linux-2.6.20-mm1/drivers/mtd/mtd_blkdevs.c 2007-02-17 00:43:31.0
+0100
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
static LIST_HEAD(blktrans_majors);
@@ -113,6 +114,8 @@ static int mtd_blktrans_thread(void *arg
schedule();
remove_wait_queue(>blkcore_priv->thread_wq, );
+ try_to_freeze();
+
spin_lock_irq(rq->queue_lock);
continue;
Index: linux-2.6.20-mm1/drivers/usb/storage/usb.c
===
--- linux-2.6.20-mm1.orig/drivers/usb/storage/usb.c 2007-02-17
00:43:19.0 +0100
+++ linux-2.6.20-mm1/drivers/usb/storage/usb.c 2007-02-17 11:39:00.0
+0100
@@ -304,6 +304,8 @@ static int usb_stor_control_thread(void
current->flags |= PF_NOFREEZE;
for(;;) {
+ try_to_freeze();
+
US_DEBUGP("*** thread sleeping.\n");
if(down_interruptible(>sema))
break;
Index: linux-2.6.20-mm1/drivers/ieee1394/ieee1394_core.c
===
---
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Thursday, 15 February 2007 15:25, Gautham R Shenoy wrote:
On Thu, Feb 15, 2007 at 02:31:08PM +0100, Rafael J. Wysocki wrote:
So I think tonight I'll start adding try_to_freeze() to the kernel threads
that
set PF_NOFREEZE.
cool! While you are at it, let me try to enhance the freezer api's
to incorporate the PFE_* flags.
Here's a patch that adds try_to_freeze() to all kernel threads that didn't call
it before. It shouldn't change the behavior of the threads in question, since
they won't be frozen because the are flagged as PF_NOFREEZE (of course
we are going to change this later).
Compile-tested on x86_64 with allmodconfig.
Pavel, do you think we can remove the PF_NOFREEZE from bluetooth, BTW?
arch/i386/kernel/apm.c |2 ++
drivers/block/loop.c|2 ++
drivers/char/apm-emulation.c|3 +++
drivers/ieee1394/ieee1394_core.c|3 +++
drivers/md/md.c |2 ++
drivers/mmc/mmc_queue.c |3 +++
drivers/mtd/mtd_blkdevs.c |3 +++
drivers/scsi/libsas/sas_scsi_host.c |3 +++
drivers/scsi/scsi_error.c |3 +++
drivers/usb/storage/usb.c |2 ++
kernel/rcutorture.c |2 ++
kernel/softirq.c|2 ++
kernel/softlockup.c |2 ++
kernel/workqueue.c |3 +--
net/bluetooth/bnep/core.c |5 -
net/bluetooth/cmtp/core.c |3 +++
net/bluetooth/hidp/core.c |3 +++
net/bluetooth/rfcomm/core.c |3 +++
18 files changed, 46 insertions(+), 3 deletions(-)
Index: linux-2.6.20-mm1/arch/i386/kernel/apm.c
===
--- linux-2.6.20-mm1.orig/arch/i386/kernel/apm.c2007-02-17
00:43:19.0 +0100
+++ linux-2.6.20-mm1/arch/i386/kernel/apm.c 2007-02-17 00:43:31.0
+0100
@@ -227,6 +227,7 @@
#include linux/dmi.h
#include linux/suspend.h
#include linux/kthread.h
+#include linux/freezer.h
#include asm/system.h
#include asm/uaccess.h
@@ -1402,6 +1403,7 @@ static void apm_mainloop(void)
add_wait_queue(apm_waitqueue, wait);
set_current_state(TASK_INTERRUPTIBLE);
for (;;) {
+ try_to_freeze();
schedule_timeout(APM_CHECK_TIMEOUT);
if (kthread_should_stop())
break;
Index: linux-2.6.20-mm1/drivers/md/md.c
===
--- linux-2.6.20-mm1.orig/drivers/md/md.c 2007-02-17 00:43:19.0
+0100
+++ linux-2.6.20-mm1/drivers/md/md.c2007-02-17 00:43:31.0 +0100
@@ -4513,6 +4513,8 @@ static int md_thread(void * arg)
|| kthread_should_stop(),
thread-timeout);
+ try_to_freeze();
+
clear_bit(THREAD_WAKEUP, thread-flags);
thread-run(thread-mddev);
Index: linux-2.6.20-mm1/drivers/mmc/mmc_queue.c
===
--- linux-2.6.20-mm1.orig/drivers/mmc/mmc_queue.c 2007-02-17
00:43:19.0 +0100
+++ linux-2.6.20-mm1/drivers/mmc/mmc_queue.c2007-02-17 00:43:31.0
+0100
@@ -11,6 +11,7 @@
#include linux/module.h
#include linux/blkdev.h
#include linux/kthread.h
+#include linux/freezer.h
#include linux/mmc/card.h
#include linux/mmc/host.h
@@ -70,6 +71,8 @@ static int mmc_queue_thread(void *d)
do {
struct request *req = NULL;
+ try_to_freeze();
+
spin_lock_irq(q-queue_lock);
set_current_state(TASK_INTERRUPTIBLE);
req = elv_next_request(q);
Index: linux-2.6.20-mm1/drivers/mtd/mtd_blkdevs.c
===
--- linux-2.6.20-mm1.orig/drivers/mtd/mtd_blkdevs.c 2007-02-17
00:43:19.0 +0100
+++ linux-2.6.20-mm1/drivers/mtd/mtd_blkdevs.c 2007-02-17 00:43:31.0
+0100
@@ -20,6 +20,7 @@
#include linux/hdreg.h
#include linux/init.h
#include linux/mutex.h
+#include linux/freezer.h
#include asm/uaccess.h
static LIST_HEAD(blktrans_majors);
@@ -113,6 +114,8 @@ static int mtd_blktrans_thread(void *arg
schedule();
remove_wait_queue(tr-blkcore_priv-thread_wq, wait);
+ try_to_freeze();
+
spin_lock_irq(rq-queue_lock);
continue;
Index: linux-2.6.20-mm1/drivers/usb/storage/usb.c
===
--- linux-2.6.20-mm1.orig/drivers/usb/storage/usb.c 2007-02-17
00:43:19.0 +0100
+++ linux-2.6.20-mm1/drivers/usb/storage/usb.c 2007-02-17 11:39:00.0
+0100
@@ -304,6 +304,8 @@ static int usb_stor_control_thread(void
current-flags |= PF_NOFREEZE;
for(;;) {
+ try_to_freeze();
+
US_DEBUGP(***
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
Rafael, I am trying to understand try_to_freeze_tasks(), and I have a
couple of questions.
static inline int is_user_space(struct task_struct *p)
{
return p-mm !(p-flags PF_BORROWED_MM);
}
This doesn't look right. First, an exiting task has -mm == NULL after
do_exit()-exit_mm(). Probably not a problem. However, PF_BORROWED_MM
check is racy without task_lock(), so we can have a false positive as
well. Is it ok? We can freeze aio_wq prematurely.
try_to_freeze_tasks:
do_each_thread(g, p) {
if (p-state == TASK_TRACED frozen(p-parent)) {
Why we are doing this check outside of if (is_user_space(p)) ?
Not a bug of course, but looks strange.
cancel_freezing(p);
continue;
Is it right? Shouldn't we increment todo counter?
}
if (is_user_space(p)) {
if (!freeze_user_space)
continue;
/* Freeze the task unless there is a vfork
* completion pending
*/
if (!p-vfork_done)
freeze_process(p);
Racy. do_fork(CLONE_VFORK) first does copy_process() which puts 'p' on
the task list and unlocks tasklist_lock. This means that 'p' is visible
to try_to_freeze_tasks(), and p-vfork_done == NULL. try_to_freeze_tasks()
sets TIF_FREEZE.
Now, do_fork() continues, sets -vfork_done, p goes to user space, notices
the fake signal and goes to refrigerator while its parent is blocked on
struct completion vfork. Freezing failed.
So, shouldn't we do
if (p-vfork_done)
cancel_freezing(p);
instead?
Thanks,
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Saturday, 17 February 2007 22:34, Oleg Nesterov wrote:
Rafael, I am trying to understand try_to_freeze_tasks(), and I have a
couple of questions.
static inline int is_user_space(struct task_struct *p)
{
return p-mm !(p-flags PF_BORROWED_MM);
}
This doesn't look right. First, an exiting task has -mm == NULL after
do_exit()-exit_mm(). Probably not a problem. However, PF_BORROWED_MM
check is racy without task_lock(), so we can have a false positive as
well. Is it ok? We can freeze aio_wq prematurely.
Right now aio_wq is not freezeable (PF_NOFREEZE).
try_to_freeze_tasks:
do_each_thread(g, p) {
if (p-state == TASK_TRACED frozen(p-parent)) {
Why we are doing this check outside of if (is_user_space(p)) ?
Not a bug of course, but looks strange.
For no particular reason.
cancel_freezing(p);
continue;
Is it right? Shouldn't we increment todo counter?
No. It would be wrong to do that, because TASK_TRACED tasks with frozen
parents cannot be frozen any further.
}
if (is_user_space(p)) {
if (!freeze_user_space)
continue;
/* Freeze the task unless there is a vfork
* completion pending
*/
if (!p-vfork_done)
freeze_process(p);
Racy. do_fork(CLONE_VFORK) first does copy_process() which puts 'p' on
the task list and unlocks tasklist_lock. This means that 'p' is visible
to try_to_freeze_tasks(), and p-vfork_done == NULL. try_to_freeze_tasks()
sets TIF_FREEZE.
Now, do_fork() continues, sets -vfork_done, p goes to user space, notices
the fake signal and goes to refrigerator while its parent is blocked on
struct completion vfork. Freezing failed.
You are right, but this has never happened, AFAICS.
So, shouldn't we do
if (p-vfork_done)
cancel_freezing(p);
instead?
I don't think so. If p hasn't got TIF_FREEZE set yet or it has already been
frozen, cancel_freezing(p) is a noop.
Alternatively, we can move the check into refrigerator(), like this:
---
kernel/power/process.c | 21 +++--
1 file changed, 7 insertions(+), 14 deletions(-)
Index: linux-2.6.20-git13/kernel/power/process.c
===
--- linux-2.6.20-git13.orig/kernel/power/process.c
+++ linux-2.6.20-git13/kernel/power/process.c
@@ -39,6 +39,11 @@ void refrigerator(void)
/* Hmm, should we be allowed to suspend when there are realtime
processes around? */
long save;
+
+ /* Freeze the task unless there is a vfork completion pending */
+ if (current-vfork_done)
+ return;
+
save = current-state;
pr_debug(%s entered refrigerator\n, current-comm);
@@ -112,22 +117,10 @@ static unsigned int try_to_freeze_tasks(
cancel_freezing(p);
continue;
}
- if (is_user_space(p)) {
- if (!freeze_user_space)
- continue;
-
- /* Freeze the task unless there is a vfork
-* completion pending
-*/
- if (!p-vfork_done)
- freeze_process(p);
- } else {
- if (freeze_user_space)
- continue;
-
+ if (is_user_space(p) == !!freeze_user_space) {
freeze_process(p);
+ todo++;
}
- todo++;
} while_each_thread(g, p);
read_unlock(tasklist_lock);
yield();/* Yield is okay here */
-
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On 02/17, Rafael J. Wysocki wrote:
On Saturday, 17 February 2007 22:34, Oleg Nesterov wrote:
static inline int is_user_space(struct task_struct *p)
{
return p-mm !(p-flags PF_BORROWED_MM);
}
This doesn't look right. First, an exiting task has -mm == NULL after
do_exit()-exit_mm(). Probably not a problem. However, PF_BORROWED_MM
check is racy without task_lock(), so we can have a false positive as
well. Is it ok? We can freeze aio_wq prematurely.
Right now aio_wq is not freezeable (PF_NOFREEZE).
Right now yes, but we are going to change this?
cancel_freezing(p);
continue;
Is it right? Shouldn't we increment todo counter?
No. It would be wrong to do that, because TASK_TRACED tasks with frozen
parents cannot be frozen any further.
TASK_TRACED task could be woken by SIGKILL. cancel_freezing() clears TIF_FREEZE.
The task may start do_exit() when try_to_freeze_tasks() returns success.
Probably not a problem.
if (!p-vfork_done)
freeze_process(p);
Racy. do_fork(CLONE_VFORK) first does copy_process() which puts 'p' on
the task list and unlocks tasklist_lock. This means that 'p' is visible
to try_to_freeze_tasks(), and p-vfork_done == NULL. try_to_freeze_tasks()
sets TIF_FREEZE.
Now, do_fork() continues, sets -vfork_done, p goes to user space, notices
the fake signal and goes to refrigerator while its parent is blocked on
struct completion vfork. Freezing failed.
You are right, but this has never happened, AFAICS.
So, shouldn't we do
if (p-vfork_done)
cancel_freezing(p);
instead?
I don't think so. If p hasn't got TIF_FREEZE set yet or it has already been
frozen, cancel_freezing(p) is a noop.
Yes, I misread cancel_freezing(), it doesn't wake up the task if it is frozen.
Alternatively, we can move the check into refrigerator(), like this:
--- linux-2.6.20-git13.orig/kernel/power/process.c
+++ linux-2.6.20-git13/kernel/power/process.c
@@ -39,6 +39,11 @@ void refrigerator(void)
/* Hmm, should we be allowed to suspend when there are realtime
processes around? */
long save;
+
+ /* Freeze the task unless there is a vfork completion pending */
+ if (current-vfork_done)
+ return;
+
This means that current returns to user space (get_signal_to_deliver
will clear TIF_SIGPENDING) and runs. While try_to_freeze_tasks() thinks
it is frozen.
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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On 02/18, Oleg Nesterov wrote: On 02/17, Rafael J. Wysocki wrote: Alternatively, we can move the check into refrigerator(), like this: --- linux-2.6.20-git13.orig/kernel/power/process.c +++ linux-2.6.20-git13/kernel/power/process.c @@ -39,6 +39,11 @@ void refrigerator(void) /* Hmm, should we be allowed to suspend when there are realtime processes around? */ long save; + + /* Freeze the task unless there is a vfork completion pending */ + if (current-vfork_done) + return; + This means that current returns to user space (get_signal_to_deliver will clear TIF_SIGPENDING) and runs. While try_to_freeze_tasks() thinks it is frozen. Ah, sorry. I am wrong, current has no PF_FROZEN yet. However, this means that sys_vfork() makes impossible to freeze processes until child exits/execs. Not good. 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Thu, Feb 15, 2007 at 02:31:08PM +0100, Rafael J. Wysocki wrote: > > So I think tonight I'll start adding try_to_freeze() to the kernel threads > that > set PF_NOFREEZE. cool! While you are at it, let me try to enhance the freezer api's to incorporate the PFE_* flags. > > > > That would still mean going over the task list twice. > > Yes, but I think this is inevitable anyway, because we have moved the > disabling of nonboot CPUs after the suspending of devices (for > ACPI-related reasons). > > Currently, we have, roughly: > > freeze_processes(); > shrink_memory(); (swsusp only) > suspend_devices(); > disable_nonboot_cpus(); > suspend > > and the reverse during the resume. > > Still, the second pass will be quick, since the majority of tasks are frozen > when disable_nonboot_cpus() is called. Ok. That's fine by me. Lets get it working first. We can always optimize it later :-) > > > > Cool! Lets get started then ;-) > > No problem with that. ;-) > > Speaking of the classification, do you think it would be practical to use > some kind of "freezing levels"? I mean, for each task we can define the > "freezing level" at which it should be frozen and each user of the freezer > can call it with a specific "freezing level" as a parameter. Of course for > this purpose the tasks frozen at level 1 have to be a subset of the tasks > frozen at level 2 etc. and I'm not sure if this requirement can be satisfied. A freeze hierarchy! I hope this freeze_level parameter ain't an alternative to the PFE_* flags because then a task would be in a dilemma as to what freezing level it should be at, if it wants to be frozen for lets say kprobes but not for cpu-hotplug. Cpu-hotplug and kprobes may not have a dependency like the one that exists between cpu-hotplug and suspend. So, at this moment, even I am not sure if there is a need for the hierarchy. > > Rafael Thanks and Regards gautham. -- Gautham R Shenoy Linux Technology Center IBM India. "Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless!" - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Thursday, 15 February 2007 13:20, Gautham R Shenoy wrote: > On Thu, Feb 15, 2007 at 09:09:51AM +0100, Rafael J. Wysocki wrote: > > > > > > Why should we make sure that PF_NOFREEZE tasks are also frozen for > > > cpu hotplug? Instead, we can create an infrastructure which allows > > > threads to > > > specify for the scenarios they would want to be excempted from freeze. > > > Something like what Paul has suggested in > > > http://lkml.org/lkml/2007/1/31/323. That way, threads which have nothing > > > to do with the online_cpu_map or with handling of cpu-hotplug events can > > > mark themselves to be exempted from being frozen for cpu hotplug. > > > > I think all kernel threads should call try_to_freeze() in suitable places > > anyway if we are going to use the freezer for anything more than just the > > suspend. In other words, they all should be _able_ to freeze if necessary. > > Yeah! I agree. I misunderstood your earlier point. I thought you were > hinting at freezing *everyone* while doing a cpu hotplug. So I think tonight I'll start adding try_to_freeze() to the kernel threads that set PF_NOFREEZE. > > > > > > Once this is achieved, it's all about classifying the threads into > > > according to their NO_FREEZE needs :) > > > > Yes, but I think it's just a generalization of ingoring PF_NOFREEZE. > > If all kernel threads are able to freeze, we can mark them as "freeze for > > CPU > > hotplug" or "freeze for kprobes", or "freeze for suspend" etc. and call the > > freezer with the appropriate parameter. > > > > BTW, what happens to a process running on a CPU being removed? > > > > We call stop_machine_run in _cpu_down which schedules an idle thread on > the cpu to be removed. Once the idle thread runs, we call __cpu_die and > subsequently the scheduler performs task migration while handling > the CPU_DEAD notification (see migration_call in sched.c) Ah, thanks for the explanation. > > > > 2) We have to change the PM code to stop using CPU hotplug for disabling > > > > nonboot CPUs. ;-) > > > > > > Just wondering, how hard is that ? > > > > Hmmm. In fact the problem is that the suspend code freezes tasks and then > > calls disable_nonboot_cpus() which uses (_)cpu_down/up(). In principle we > > could make disable_nonboot_cpus() call some lower-level routines to avoid > > the > > freezing of tasks, _but_ the suspend code may freeze too few tasks (ie. we > > may > > want to freeze more tasks for the CPU hotplug). Thus I think we should do > > something like this: > > > > suspend:CPU hotplug: > > freeze_processes(SUSPEND) ... > > ... freeze_processes(CPU_HOTPLUG) > > ... ... > > ... thaw_processes(CPU_HOTPLUG) > > thaw_processes(SUSPEND) ... > > > > so freeze_processes() should be reentrant, at least for different values of > > the argument. > > > > That would still mean going over the task list twice. Yes, but I think this is inevitable anyway, because we have moved the disabling of nonboot CPUs after the suspending of devices (for ACPI-related reasons). Currently, we have, roughly: freeze_processes(); shrink_memory(); (swsusp only) suspend_devices(); disable_nonboot_cpus(); suspend and the reverse during the resume. Still, the second pass will be quick, since the majority of tasks are frozen when disable_nonboot_cpus() is called. > How if we have > > freeze_process(SUSPEND|CPU_HOTPLUG); > perform_pre_hotplug_suspend(); > primitive_cpu_down/_up(); > perform_post_hotplug_suspend(); > > Does this look like a good thing to you? > > All in all, I think we should start from modifying the freezer and the > > classification of processes with respect to the freezing. > > > > Cool! Lets get started then ;-) No problem with that. ;-) Speaking of the classification, do you think it would be practical to use some kind of "freezing levels"? I mean, for each task we can define the "freezing level" at which it should be frozen and each user of the freezer can call it with a specific "freezing level" as a parameter. Of course for this purpose the tasks frozen at level 1 have to be a subset of the tasks frozen at level 2 etc. and I'm not sure if this requirement can be satisfied. Rafael - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Thu, Feb 15, 2007 at 09:09:51AM +0100, Rafael J. Wysocki wrote: > > > > Why should we make sure that PF_NOFREEZE tasks are also frozen for > > cpu hotplug? Instead, we can create an infrastructure which allows threads > > to > > specify for the scenarios they would want to be excempted from freeze. > > Something like what Paul has suggested in > > http://lkml.org/lkml/2007/1/31/323. That way, threads which have nothing > > to do with the online_cpu_map or with handling of cpu-hotplug events can > > mark themselves to be exempted from being frozen for cpu hotplug. > > I think all kernel threads should call try_to_freeze() in suitable places > anyway if we are going to use the freezer for anything more than just the > suspend. In other words, they all should be _able_ to freeze if necessary. Yeah! I agree. I misunderstood your earlier point. I thought you were hinting at freezing *everyone* while doing a cpu hotplug. > > > Once this is achieved, it's all about classifying the threads into > > according to their NO_FREEZE needs :) > > Yes, but I think it's just a generalization of ingoring PF_NOFREEZE. > If all kernel threads are able to freeze, we can mark them as "freeze for CPU > hotplug" or "freeze for kprobes", or "freeze for suspend" etc. and call the > freezer with the appropriate parameter. > > BTW, what happens to a process running on a CPU being removed? > We call stop_machine_run in _cpu_down which schedules an idle thread on the cpu to be removed. Once the idle thread runs, we call __cpu_die and subsequently the scheduler performs task migration while handling the CPU_DEAD notification (see migration_call in sched.c) > > > 2) We have to change the PM code to stop using CPU hotplug for disabling > > > nonboot CPUs. ;-) > > > > Just wondering, how hard is that ? > > Hmmm. In fact the problem is that the suspend code freezes tasks and then > calls disable_nonboot_cpus() which uses (_)cpu_down/up(). In principle we > could make disable_nonboot_cpus() call some lower-level routines to avoid the > freezing of tasks, _but_ the suspend code may freeze too few tasks (ie. we may > want to freeze more tasks for the CPU hotplug). Thus I think we should do > something like this: > > suspend: CPU hotplug: > freeze_processes(SUSPEND) ... > ... freeze_processes(CPU_HOTPLUG) > ... ... > ... thaw_processes(CPU_HOTPLUG) > thaw_processes(SUSPEND) ... > > so freeze_processes() should be reentrant, at least for different values of > the argument. > That would still mean going over the task list twice. How if we have freeze_process(SUSPEND|CPU_HOTPLUG); perform_pre_hotplug_suspend(); primitive_cpu_down/_up(); perform_post_hotplug_suspend(); Does this look like a good thing to you? > All in all, I think we should start from modifying the freezer and the > classification of processes with respect to the freezing. > Cool! Lets get started then ;-) > Greetings, > Rafael -- Gautham R Shenoy Linux Technology Center IBM India. "Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless!" - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Thursday, 15 February 2007 07:34, Gautham R Shenoy wrote: > On Wed, Feb 14, 2007 at 10:43:35PM +0100, Rafael J. Wysocki wrote: > > Hi, > > > > On Wednesday, 14 February 2007 15:40, Gautham R Shenoy wrote: > > > Hello Everybody, > > > > > > This is an experiment towards process_freezer based implementation > > > of cpu-hotplug. This is mainly based on ideas of Andrew Morton, > > > Ingo Molnar and Paul Mckenney featured in the discussion > > > http://lkml.org/lkml/2007/1/31/323. > > > > > > This is an absolute bare-minimal implementation to check the feasibility > > > of using process freezer for cpu-hotplug. > > > > > > The patchset comprises of four patches. > > > o PATCH 1/4: Core implementation of freezer-based-hotplug. > > > o PATCH 2/4: Revert changes to workqueue to make it work with the > > > freezer-cpu-hotplug. > > > o PATCH 3/4: Eliminate hotcpu subsystem mutexes from sched and slab. > > > o PATCH 4/4: Eliminate lock_cpu_hotplug from the kernel. > > > > I think two things are missing: > > > > 1) We should make sure there are not PF_NOFREEZE tasks running when a CPU > > is removed (when one is added probably too). For this purpose we can add a > > parameter to freeze_processes() that will tell it to ignore PF_NOFREEZE, but > > at the same time we'll have to change all kernel threads that set > > PF_NOFREEZE > > to call try_to_freeze() anyway. I can do that, but it will take me a > > couple of > > days. > > Why should we make sure that PF_NOFREEZE tasks are also frozen for > cpu hotplug? Instead, we can create an infrastructure which allows threads to > specify for the scenarios they would want to be excempted from freeze. > Something like what Paul has suggested in > http://lkml.org/lkml/2007/1/31/323. That way, threads which have nothing > to do with the online_cpu_map or with handling of cpu-hotplug events can > mark themselves to be exempted from being frozen for cpu hotplug. I think all kernel threads should call try_to_freeze() in suitable places anyway if we are going to use the freezer for anything more than just the suspend. In other words, they all should be _able_ to freeze if necessary. > Once this is achieved, it's all about classifying the threads into > according to their NO_FREEZE needs :) Yes, but I think it's just a generalization of ingoring PF_NOFREEZE. If all kernel threads are able to freeze, we can mark them as "freeze for CPU hotplug" or "freeze for kprobes", or "freeze for suspend" etc. and call the freezer with the appropriate parameter. BTW, what happens to a process running on a CPU being removed? > > 2) We have to change the PM code to stop using CPU hotplug for disabling > > nonboot CPUs. ;-) > > Just wondering, how hard is that ? Hmmm. In fact the problem is that the suspend code freezes tasks and then calls disable_nonboot_cpus() which uses (_)cpu_down/up(). In principle we could make disable_nonboot_cpus() call some lower-level routines to avoid the freezing of tasks, _but_ the suspend code may freeze too few tasks (ie. we may want to freeze more tasks for the CPU hotplug). Thus I think we should do something like this: suspend:CPU hotplug: freeze_processes(SUSPEND) ... ... freeze_processes(CPU_HOTPLUG) ... ... ... thaw_processes(CPU_HOTPLUG) thaw_processes(SUSPEND) ... so freeze_processes() should be reentrant, at least for different values of the argument. All in all, I think we should start from modifying the freezer and the classification of processes with respect to the freezing. Greetings, Rafael - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Thursday, 15 February 2007 07:34, Gautham R Shenoy wrote: On Wed, Feb 14, 2007 at 10:43:35PM +0100, Rafael J. Wysocki wrote: Hi, On Wednesday, 14 February 2007 15:40, Gautham R Shenoy wrote: Hello Everybody, This is an experiment towards process_freezer based implementation of cpu-hotplug. This is mainly based on ideas of Andrew Morton, Ingo Molnar and Paul Mckenney featured in the discussion http://lkml.org/lkml/2007/1/31/323. This is an absolute bare-minimal implementation to check the feasibility of using process freezer for cpu-hotplug. The patchset comprises of four patches. o PATCH 1/4: Core implementation of freezer-based-hotplug. o PATCH 2/4: Revert changes to workqueue to make it work with the freezer-cpu-hotplug. o PATCH 3/4: Eliminate hotcpu subsystem mutexes from sched and slab. o PATCH 4/4: Eliminate lock_cpu_hotplug from the kernel. I think two things are missing: 1) We should make sure there are not PF_NOFREEZE tasks running when a CPU is removed (when one is added probably too). For this purpose we can add a parameter to freeze_processes() that will tell it to ignore PF_NOFREEZE, but at the same time we'll have to change all kernel threads that set PF_NOFREEZE to call try_to_freeze() anyway. I can do that, but it will take me a couple of days. Why should we make sure that PF_NOFREEZE tasks are also frozen for cpu hotplug? Instead, we can create an infrastructure which allows threads to specify for the scenarios they would want to be excempted from freeze. Something like what Paul has suggested in http://lkml.org/lkml/2007/1/31/323. That way, threads which have nothing to do with the online_cpu_map or with handling of cpu-hotplug events can mark themselves to be exempted from being frozen for cpu hotplug. I think all kernel threads should call try_to_freeze() in suitable places anyway if we are going to use the freezer for anything more than just the suspend. In other words, they all should be _able_ to freeze if necessary. Once this is achieved, it's all about classifying the threads into according to their NO_FREEZE needs :) Yes, but I think it's just a generalization of ingoring PF_NOFREEZE. If all kernel threads are able to freeze, we can mark them as freeze for CPU hotplug or freeze for kprobes, or freeze for suspend etc. and call the freezer with the appropriate parameter. BTW, what happens to a process running on a CPU being removed? 2) We have to change the PM code to stop using CPU hotplug for disabling nonboot CPUs. ;-) Just wondering, how hard is that ? Hmmm. In fact the problem is that the suspend code freezes tasks and then calls disable_nonboot_cpus() which uses (_)cpu_down/up(). In principle we could make disable_nonboot_cpus() call some lower-level routines to avoid the freezing of tasks, _but_ the suspend code may freeze too few tasks (ie. we may want to freeze more tasks for the CPU hotplug). Thus I think we should do something like this: suspend:CPU hotplug: freeze_processes(SUSPEND) ... ... freeze_processes(CPU_HOTPLUG) ... ... ... thaw_processes(CPU_HOTPLUG) thaw_processes(SUSPEND) ... so freeze_processes() should be reentrant, at least for different values of the argument. All in all, I think we should start from modifying the freezer and the classification of processes with respect to the freezing. Greetings, Rafael - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Thu, Feb 15, 2007 at 09:09:51AM +0100, Rafael J. Wysocki wrote: Why should we make sure that PF_NOFREEZE tasks are also frozen for cpu hotplug? Instead, we can create an infrastructure which allows threads to specify for the scenarios they would want to be excempted from freeze. Something like what Paul has suggested in http://lkml.org/lkml/2007/1/31/323. That way, threads which have nothing to do with the online_cpu_map or with handling of cpu-hotplug events can mark themselves to be exempted from being frozen for cpu hotplug. I think all kernel threads should call try_to_freeze() in suitable places anyway if we are going to use the freezer for anything more than just the suspend. In other words, they all should be _able_ to freeze if necessary. Yeah! I agree. I misunderstood your earlier point. I thought you were hinting at freezing *everyone* while doing a cpu hotplug. Once this is achieved, it's all about classifying the threads into according to their NO_FREEZE needs :) Yes, but I think it's just a generalization of ingoring PF_NOFREEZE. If all kernel threads are able to freeze, we can mark them as freeze for CPU hotplug or freeze for kprobes, or freeze for suspend etc. and call the freezer with the appropriate parameter. BTW, what happens to a process running on a CPU being removed? We call stop_machine_run in _cpu_down which schedules an idle thread on the cpu to be removed. Once the idle thread runs, we call __cpu_die and subsequently the scheduler performs task migration while handling the CPU_DEAD notification (see migration_call in sched.c) 2) We have to change the PM code to stop using CPU hotplug for disabling nonboot CPUs. ;-) Just wondering, how hard is that ? Hmmm. In fact the problem is that the suspend code freezes tasks and then calls disable_nonboot_cpus() which uses (_)cpu_down/up(). In principle we could make disable_nonboot_cpus() call some lower-level routines to avoid the freezing of tasks, _but_ the suspend code may freeze too few tasks (ie. we may want to freeze more tasks for the CPU hotplug). Thus I think we should do something like this: suspend: CPU hotplug: freeze_processes(SUSPEND) ... ... freeze_processes(CPU_HOTPLUG) ... ... ... thaw_processes(CPU_HOTPLUG) thaw_processes(SUSPEND) ... so freeze_processes() should be reentrant, at least for different values of the argument. That would still mean going over the task list twice. How if we have freeze_process(SUSPEND|CPU_HOTPLUG); perform_pre_hotplug_suspend(); primitive_cpu_down/_up(); perform_post_hotplug_suspend(); Does this look like a good thing to you? All in all, I think we should start from modifying the freezer and the classification of processes with respect to the freezing. Cool! Lets get started then ;-) Greetings, Rafael -- Gautham R Shenoy Linux Technology Center IBM India. Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless! - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Thursday, 15 February 2007 13:20, Gautham R Shenoy wrote: On Thu, Feb 15, 2007 at 09:09:51AM +0100, Rafael J. Wysocki wrote: Why should we make sure that PF_NOFREEZE tasks are also frozen for cpu hotplug? Instead, we can create an infrastructure which allows threads to specify for the scenarios they would want to be excempted from freeze. Something like what Paul has suggested in http://lkml.org/lkml/2007/1/31/323. That way, threads which have nothing to do with the online_cpu_map or with handling of cpu-hotplug events can mark themselves to be exempted from being frozen for cpu hotplug. I think all kernel threads should call try_to_freeze() in suitable places anyway if we are going to use the freezer for anything more than just the suspend. In other words, they all should be _able_ to freeze if necessary. Yeah! I agree. I misunderstood your earlier point. I thought you were hinting at freezing *everyone* while doing a cpu hotplug. So I think tonight I'll start adding try_to_freeze() to the kernel threads that set PF_NOFREEZE. Once this is achieved, it's all about classifying the threads into according to their NO_FREEZE needs :) Yes, but I think it's just a generalization of ingoring PF_NOFREEZE. If all kernel threads are able to freeze, we can mark them as freeze for CPU hotplug or freeze for kprobes, or freeze for suspend etc. and call the freezer with the appropriate parameter. BTW, what happens to a process running on a CPU being removed? We call stop_machine_run in _cpu_down which schedules an idle thread on the cpu to be removed. Once the idle thread runs, we call __cpu_die and subsequently the scheduler performs task migration while handling the CPU_DEAD notification (see migration_call in sched.c) Ah, thanks for the explanation. 2) We have to change the PM code to stop using CPU hotplug for disabling nonboot CPUs. ;-) Just wondering, how hard is that ? Hmmm. In fact the problem is that the suspend code freezes tasks and then calls disable_nonboot_cpus() which uses (_)cpu_down/up(). In principle we could make disable_nonboot_cpus() call some lower-level routines to avoid the freezing of tasks, _but_ the suspend code may freeze too few tasks (ie. we may want to freeze more tasks for the CPU hotplug). Thus I think we should do something like this: suspend:CPU hotplug: freeze_processes(SUSPEND) ... ... freeze_processes(CPU_HOTPLUG) ... ... ... thaw_processes(CPU_HOTPLUG) thaw_processes(SUSPEND) ... so freeze_processes() should be reentrant, at least for different values of the argument. That would still mean going over the task list twice. Yes, but I think this is inevitable anyway, because we have moved the disabling of nonboot CPUs after the suspending of devices (for ACPI-related reasons). Currently, we have, roughly: freeze_processes(); shrink_memory(); (swsusp only) suspend_devices(); disable_nonboot_cpus(); suspend and the reverse during the resume. Still, the second pass will be quick, since the majority of tasks are frozen when disable_nonboot_cpus() is called. How if we have freeze_process(SUSPEND|CPU_HOTPLUG); perform_pre_hotplug_suspend(); primitive_cpu_down/_up(); perform_post_hotplug_suspend(); Does this look like a good thing to you? All in all, I think we should start from modifying the freezer and the classification of processes with respect to the freezing. Cool! Lets get started then ;-) No problem with that. ;-) Speaking of the classification, do you think it would be practical to use some kind of freezing levels? I mean, for each task we can define the freezing level at which it should be frozen and each user of the freezer can call it with a specific freezing level as a parameter. Of course for this purpose the tasks frozen at level 1 have to be a subset of the tasks frozen at level 2 etc. and I'm not sure if this requirement can be satisfied. Rafael - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Thu, Feb 15, 2007 at 02:31:08PM +0100, Rafael J. Wysocki wrote: So I think tonight I'll start adding try_to_freeze() to the kernel threads that set PF_NOFREEZE. cool! While you are at it, let me try to enhance the freezer api's to incorporate the PFE_* flags. That would still mean going over the task list twice. Yes, but I think this is inevitable anyway, because we have moved the disabling of nonboot CPUs after the suspending of devices (for ACPI-related reasons). Currently, we have, roughly: freeze_processes(); shrink_memory(); (swsusp only) suspend_devices(); disable_nonboot_cpus(); suspend and the reverse during the resume. Still, the second pass will be quick, since the majority of tasks are frozen when disable_nonboot_cpus() is called. Ok. That's fine by me. Lets get it working first. We can always optimize it later :-) Cool! Lets get started then ;-) No problem with that. ;-) Speaking of the classification, do you think it would be practical to use some kind of freezing levels? I mean, for each task we can define the freezing level at which it should be frozen and each user of the freezer can call it with a specific freezing level as a parameter. Of course for this purpose the tasks frozen at level 1 have to be a subset of the tasks frozen at level 2 etc. and I'm not sure if this requirement can be satisfied. A freeze hierarchy! I hope this freeze_level parameter ain't an alternative to the PFE_* flags because then a task would be in a dilemma as to what freezing level it should be at, if it wants to be frozen for lets say kprobes but not for cpu-hotplug. Cpu-hotplug and kprobes may not have a dependency like the one that exists between cpu-hotplug and suspend. So, at this moment, even I am not sure if there is a need for the hierarchy. Rafael Thanks and Regards gautham. -- Gautham R Shenoy Linux Technology Center IBM India. Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless! - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Wed, Feb 14, 2007 at 10:43:35PM +0100, Rafael J. Wysocki wrote: > Hi, > > On Wednesday, 14 February 2007 15:40, Gautham R Shenoy wrote: > > Hello Everybody, > > > > This is an experiment towards process_freezer based implementation > > of cpu-hotplug. This is mainly based on ideas of Andrew Morton, > > Ingo Molnar and Paul Mckenney featured in the discussion > > http://lkml.org/lkml/2007/1/31/323. > > > > This is an absolute bare-minimal implementation to check the feasibility > > of using process freezer for cpu-hotplug. > > > > The patchset comprises of four patches. > > o PATCH 1/4: Core implementation of freezer-based-hotplug. > > o PATCH 2/4: Revert changes to workqueue to make it work with the > > freezer-cpu-hotplug. > > o PATCH 3/4: Eliminate hotcpu subsystem mutexes from sched and slab. > > o PATCH 4/4: Eliminate lock_cpu_hotplug from the kernel. > > I think two things are missing: > > 1) We should make sure there are not PF_NOFREEZE tasks running when a CPU > is removed (when one is added probably too). For this purpose we can add a > parameter to freeze_processes() that will tell it to ignore PF_NOFREEZE, but > at the same time we'll have to change all kernel threads that set PF_NOFREEZE > to call try_to_freeze() anyway. I can do that, but it will take me a couple > of > days. Why should we make sure that PF_NOFREEZE tasks are also frozen for cpu hotplug? Instead, we can create an infrastructure which allows threads to specify for the scenarios they would want to be excempted from freeze. Something like what Paul has suggested in http://lkml.org/lkml/2007/1/31/323. That way, threads which have nothing to do with the online_cpu_map or with handling of cpu-hotplug events can mark themselves to be exempted from being frozen for cpu hotplug. Once this is achieved, it's all about classifying the threads into according to their NO_FREEZE needs :) > > 2) We have to change the PM code to stop using CPU hotplug for disabling > nonboot CPUs. ;-) Just wondering, how hard is that ? thanks gautham. -- Gautham R Shenoy Linux Technology Center IBM India. "Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless!" - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
Hi, On Wednesday, 14 February 2007 15:40, Gautham R Shenoy wrote: > Hello Everybody, > > This is an experiment towards process_freezer based implementation > of cpu-hotplug. This is mainly based on ideas of Andrew Morton, > Ingo Molnar and Paul Mckenney featured in the discussion > http://lkml.org/lkml/2007/1/31/323. > > This is an absolute bare-minimal implementation to check the feasibility > of using process freezer for cpu-hotplug. > > The patchset comprises of four patches. > o PATCH 1/4: Core implementation of freezer-based-hotplug. > o PATCH 2/4: Revert changes to workqueue to make it work with the > freezer-cpu-hotplug. > o PATCH 3/4: Eliminate hotcpu subsystem mutexes from sched and slab. > o PATCH 4/4: Eliminate lock_cpu_hotplug from the kernel. I think two things are missing: 1) We should make sure there are not PF_NOFREEZE tasks running when a CPU is removed (when one is added probably too). For this purpose we can add a parameter to freeze_processes() that will tell it to ignore PF_NOFREEZE, but at the same time we'll have to change all kernel threads that set PF_NOFREEZE to call try_to_freeze() anyway. I can do that, but it will take me a couple of days. 2) We have to change the PM code to stop using CPU hotplug for disabling nonboot CPUs. ;-) Greetings, Rafael - 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/
[RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
Hello Everybody, This is an experiment towards process_freezer based implementation of cpu-hotplug. This is mainly based on ideas of Andrew Morton, Ingo Molnar and Paul Mckenney featured in the discussion http://lkml.org/lkml/2007/1/31/323. This is an absolute bare-minimal implementation to check the feasibility of using process freezer for cpu-hotplug. The patchset comprises of four patches. o PATCH 1/4: Core implementation of freezer-based-hotplug. o PATCH 2/4: Revert changes to workqueue to make it work with the freezer-cpu-hotplug. o PATCH 3/4: Eliminate hotcpu subsystem mutexes from sched and slab. o PATCH 4/4: Eliminate lock_cpu_hotplug from the kernel. These patches have been stress tested on i386 SMP box, with cpu-hotplug operations in a tight-loop and make -jN (kernel compile) running parallely. The cpu hotplug operations have been pretty stable. Observation: - Certain threads like ksoftirqd/1, firmware.agent were not frozen during the hotplug operations. However, these were rare occurances. This implementation is by no means perfect or complete. Things that are yet to be done are as follows: - Most of Paul's suggestions including introduction of new states for process freezer like PFE_SUSPEND, PFE_KPROBES, PFE_HOTPLUG so that processes which are not concerned with these events can be exempted from being frozen. - Ensure the working of cpu_hotplug on all architectures on which it is supported. At present, it has been tested only on i386. - Updated documentation for cpu-hotplug. Any feedback would be greatly appreciated. Thanks and Regards gautham. -- Gautham R Shenoy Linux Technology Center IBM India. "Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless!" - 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/
[RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
Hello Everybody, This is an experiment towards process_freezer based implementation of cpu-hotplug. This is mainly based on ideas of Andrew Morton, Ingo Molnar and Paul Mckenney featured in the discussion http://lkml.org/lkml/2007/1/31/323. This is an absolute bare-minimal implementation to check the feasibility of using process freezer for cpu-hotplug. The patchset comprises of four patches. o PATCH 1/4: Core implementation of freezer-based-hotplug. o PATCH 2/4: Revert changes to workqueue to make it work with the freezer-cpu-hotplug. o PATCH 3/4: Eliminate hotcpu subsystem mutexes from sched and slab. o PATCH 4/4: Eliminate lock_cpu_hotplug from the kernel. These patches have been stress tested on i386 SMP box, with cpu-hotplug operations in a tight-loop and make -jN (kernel compile) running parallely. The cpu hotplug operations have been pretty stable. Observation: - Certain threads like ksoftirqd/1, firmware.agent were not frozen during the hotplug operations. However, these were rare occurances. This implementation is by no means perfect or complete. Things that are yet to be done are as follows: - Most of Paul's suggestions including introduction of new states for process freezer like PFE_SUSPEND, PFE_KPROBES, PFE_HOTPLUG so that processes which are not concerned with these events can be exempted from being frozen. - Ensure the working of cpu_hotplug on all architectures on which it is supported. At present, it has been tested only on i386. - Updated documentation for cpu-hotplug. Any feedback would be greatly appreciated. Thanks and Regards gautham. -- Gautham R Shenoy Linux Technology Center IBM India. Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless! - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
Hi, On Wednesday, 14 February 2007 15:40, Gautham R Shenoy wrote: Hello Everybody, This is an experiment towards process_freezer based implementation of cpu-hotplug. This is mainly based on ideas of Andrew Morton, Ingo Molnar and Paul Mckenney featured in the discussion http://lkml.org/lkml/2007/1/31/323. This is an absolute bare-minimal implementation to check the feasibility of using process freezer for cpu-hotplug. The patchset comprises of four patches. o PATCH 1/4: Core implementation of freezer-based-hotplug. o PATCH 2/4: Revert changes to workqueue to make it work with the freezer-cpu-hotplug. o PATCH 3/4: Eliminate hotcpu subsystem mutexes from sched and slab. o PATCH 4/4: Eliminate lock_cpu_hotplug from the kernel. I think two things are missing: 1) We should make sure there are not PF_NOFREEZE tasks running when a CPU is removed (when one is added probably too). For this purpose we can add a parameter to freeze_processes() that will tell it to ignore PF_NOFREEZE, but at the same time we'll have to change all kernel threads that set PF_NOFREEZE to call try_to_freeze() anyway. I can do that, but it will take me a couple of days. 2) We have to change the PM code to stop using CPU hotplug for disabling nonboot CPUs. ;-) Greetings, Rafael - 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/
Re: [RFC PATCH(Experimental) 0/4] Freezer based Cpu-hotplug
On Wed, Feb 14, 2007 at 10:43:35PM +0100, Rafael J. Wysocki wrote: Hi, On Wednesday, 14 February 2007 15:40, Gautham R Shenoy wrote: Hello Everybody, This is an experiment towards process_freezer based implementation of cpu-hotplug. This is mainly based on ideas of Andrew Morton, Ingo Molnar and Paul Mckenney featured in the discussion http://lkml.org/lkml/2007/1/31/323. This is an absolute bare-minimal implementation to check the feasibility of using process freezer for cpu-hotplug. The patchset comprises of four patches. o PATCH 1/4: Core implementation of freezer-based-hotplug. o PATCH 2/4: Revert changes to workqueue to make it work with the freezer-cpu-hotplug. o PATCH 3/4: Eliminate hotcpu subsystem mutexes from sched and slab. o PATCH 4/4: Eliminate lock_cpu_hotplug from the kernel. I think two things are missing: 1) We should make sure there are not PF_NOFREEZE tasks running when a CPU is removed (when one is added probably too). For this purpose we can add a parameter to freeze_processes() that will tell it to ignore PF_NOFREEZE, but at the same time we'll have to change all kernel threads that set PF_NOFREEZE to call try_to_freeze() anyway. I can do that, but it will take me a couple of days. Why should we make sure that PF_NOFREEZE tasks are also frozen for cpu hotplug? Instead, we can create an infrastructure which allows threads to specify for the scenarios they would want to be excempted from freeze. Something like what Paul has suggested in http://lkml.org/lkml/2007/1/31/323. That way, threads which have nothing to do with the online_cpu_map or with handling of cpu-hotplug events can mark themselves to be exempted from being frozen for cpu hotplug. Once this is achieved, it's all about classifying the threads into according to their NO_FREEZE needs :) 2) We have to change the PM code to stop using CPU hotplug for disabling nonboot CPUs. ;-) Just wondering, how hard is that ? thanks gautham. -- Gautham R Shenoy Linux Technology Center IBM India. Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless! - 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/
