* [PATCH -mm 1/7] Freezer: Read PF_BORROWED_MM in a nonracy way
2007-03-01 15:05 ` [PATCH -mm 0/7] Freezer changes (take 2) Rafael J. Wysocki
@ 2007-03-01 15:06 ` Rafael J. Wysocki
2007-03-01 15:08 ` [PATCH -mm 2/7] Freezer: Close theoretical race between refrigerator and thaw_tasks Rafael J. Wysocki
` (5 subsequent siblings)
6 siblings, 0 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2007-03-01 15:06 UTC (permalink / raw)
To: Andrew Morton
Cc: Pavel Machek, LKML, Oleg Nesterov, Aneesh Kumar,
Paul E. McKenney, Srivatsa Vaddagiri, Gautham R Shenoy
From: Rafael J. Wysocki <rjw@sisk.pl>
The reading of PF_BORROWED_MM in is_user_space() without task_lock() is racy.
Fix it.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
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 2007-02-25 14:02:55.000000000 +0100
+++ linux-2.6.20-mm2/kernel/power/process.c 2007-02-25 14:03:51.000000000 +0100
@@ -8,6 +8,7 @@
#undef DEBUG
+#include <linux/sched.h>
#include <linux/smp_lock.h>
#include <linux/interrupt.h>
#include <linux/suspend.h>
@@ -87,7 +88,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)
^ permalink raw reply [flat|nested] 46+ messages in thread* [PATCH -mm 2/7] Freezer: Close theoretical race between refrigerator and thaw_tasks
2007-03-01 15:05 ` [PATCH -mm 0/7] Freezer changes (take 2) Rafael J. Wysocki
2007-03-01 15:06 ` [PATCH -mm 1/7] Freezer: Read PF_BORROWED_MM in a nonracy way Rafael J. Wysocki
@ 2007-03-01 15:08 ` Rafael J. Wysocki
2007-03-01 16:39 ` Pavel Machek
2007-03-01 15:09 ` [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread Rafael J. Wysocki
` (4 subsequent siblings)
6 siblings, 1 reply; 46+ messages in thread
From: Rafael J. Wysocki @ 2007-03-01 15:08 UTC (permalink / raw)
To: Andrew Morton
Cc: Pavel Machek, LKML, Oleg Nesterov, Aneesh Kumar,
Paul E. McKenney, Srivatsa Vaddagiri, Gautham R Shenoy
From: Rafael J. Wysocki <rjw@sisk.pl>
If the freezing of tasks fails and a task is preempted in refrigerator() before
calling frozen_process(), then thaw_tasks() may run before this task is frozen.
In that case the task will freeze and no one will thaw it.
To fix this race we can call freezing(current) in refrigerator() along with
frozen_process(current) under the task_lock() which also should be taken in
the error path of try_to_freeze_tasks() as well as in thaw_process(). Moreover,
if thaw_process() additionally clears TIF_FREEZE for tasks that are not frozen,
we can be sure that all tasks are thawed and there are no pending "freeze"
requests after thaw_tasks() has run.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
include/linux/freezer.h | 4 ++++
kernel/power/process.c | 12 +++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
Index: linux-2.6.20-mm2/include/linux/freezer.h
===================================================================
--- linux-2.6.20-mm2.orig/include/linux/freezer.h
+++ linux-2.6.20-mm2/include/linux/freezer.h
@@ -40,11 +40,15 @@ static inline void do_not_freeze(struct
*/
static inline int thaw_process(struct task_struct *p)
{
+ task_lock(p);
if (frozen(p)) {
p->flags &= ~PF_FROZEN;
+ task_unlock(p);
wake_up_process(p);
return 1;
}
+ clear_tsk_thread_flag(p, TIF_FREEZE);
+ task_unlock(p);
return 0;
}
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
@@ -39,10 +39,18 @@ void refrigerator(void)
/* Hmm, should we be allowed to suspend when there are realtime
processes around? */
long save;
+
+ task_lock(current);
+ if (freezing(current)) {
+ frozen_process(current);
+ task_unlock(current);
+ } else {
+ task_unlock(current);
+ return;
+ }
save = current->state;
pr_debug("%s entered refrigerator\n", current->comm);
- frozen_process(current);
spin_lock_irq(¤t->sighand->siglock);
recalc_sigpending(); /* We sent fake signal, clean it up */
spin_unlock_irq(¤t->sighand->siglock);
@@ -159,10 +167,12 @@ static unsigned int try_to_freeze_tasks(
if (is_user_space(p) == !freeze_user_space)
continue;
+ task_lock(p);
if (freezeable(p) && !frozen(p))
printk(KERN_ERR " %s\n", p->comm);
cancel_freezing(p);
+ task_unlock(p);
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
}
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH -mm 2/7] Freezer: Close theoretical race between refrigerator and thaw_tasks
2007-03-01 15:08 ` [PATCH -mm 2/7] Freezer: Close theoretical race between refrigerator and thaw_tasks Rafael J. Wysocki
@ 2007-03-01 16:39 ` Pavel Machek
0 siblings, 0 replies; 46+ messages in thread
From: Pavel Machek @ 2007-03-01 16:39 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Andrew Morton, LKML, Oleg Nesterov, Aneesh Kumar,
Paul E. McKenney, Srivatsa Vaddagiri, Gautham R Shenoy
On Thu 2007-03-01 16:08:12, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> If the freezing of tasks fails and a task is preempted in refrigerator() before
> calling frozen_process(), then thaw_tasks() may run before this task is frozen.
> In that case the task will freeze and no one will thaw it.
>
> To fix this race we can call freezing(current) in refrigerator() along with
> frozen_process(current) under the task_lock() which also should be taken in
> the error path of try_to_freeze_tasks() as well as in thaw_process(). Moreover,
> if thaw_process() additionally clears TIF_FREEZE for tasks that are not frozen,
> we can be sure that all tasks are thawed and there are no pending "freeze"
> requests after thaw_tasks() has run.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
ack.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
2007-03-01 15:05 ` [PATCH -mm 0/7] Freezer changes (take 2) Rafael J. Wysocki
2007-03-01 15:06 ` [PATCH -mm 1/7] Freezer: Read PF_BORROWED_MM in a nonracy way Rafael J. Wysocki
2007-03-01 15:08 ` [PATCH -mm 2/7] Freezer: Close theoretical race between refrigerator and thaw_tasks Rafael J. Wysocki
@ 2007-03-01 15:09 ` Rafael J. Wysocki
2007-03-01 19:38 ` Anton Blanchard
2007-03-02 6:57 ` Gautham R Shenoy
2007-03-01 15:10 ` [PATCH -mm 4/7] Freezer: Remove PF_NOFREEZE from bluetooth threads Rafael J. Wysocki
` (3 subsequent siblings)
6 siblings, 2 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2007-03-01 15:09 UTC (permalink / raw)
To: Andrew Morton
Cc: Pavel Machek, LKML, Oleg Nesterov, Aneesh Kumar,
Paul E. McKenney, Srivatsa Vaddagiri, Gautham R Shenoy
From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Remove PF_NOFREEZE from the rcutorture thread, adding a try_to_freeze() call as
required.
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
kernel/rcutorture.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux-2.6.20-mm2/kernel/rcutorture.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/rcutorture.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/kernel/rcutorture.c 2007-02-25 12:49:23.000000000 +0100
@@ -46,6 +46,7 @@
#include <linux/byteorder/swabb.h>
#include <linux/stat.h>
#include <linux/srcu.h>
+#include <linux/freezer.h>
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Paul E. McKenney <paulmck@us.ibm.com> and "
@@ -585,7 +586,6 @@ rcu_torture_writer(void *arg)
VERBOSE_PRINTK_STRING("rcu_torture_writer task started");
set_user_nice(current, 19);
- current->flags |= PF_NOFREEZE;
do {
schedule_timeout_uninterruptible(1);
@@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
}
rcu_torture_current_version++;
oldbatch = cur_ops->completed();
+ try_to_freeze();
} while (!kthread_should_stop() && !fullstop);
VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
while (!kthread_should_stop())
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
2007-03-01 15:09 ` [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread Rafael J. Wysocki
@ 2007-03-01 19:38 ` Anton Blanchard
2007-03-01 19:54 ` Rafael J. Wysocki
2007-03-02 6:57 ` Gautham R Shenoy
1 sibling, 1 reply; 46+ messages in thread
From: Anton Blanchard @ 2007-03-01 19:38 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Andrew Morton, Pavel Machek, LKML, Oleg Nesterov, Aneesh Kumar,
Paul E. McKenney, Srivatsa Vaddagiri, Gautham R Shenoy
Hi,
> Remove PF_NOFREEZE from the rcutorture thread, adding a
> try_to_freeze() call as required.
...
> @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
> }
> rcu_torture_current_version++;
> oldbatch = cur_ops->completed();
> + try_to_freeze();
> } while (!kthread_should_stop() && !fullstop);
> VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> while (!kthread_should_stop())
I wonder if it makes sense to embed try_to_freeze() into the kthread
API somewhere. Short of that we should document the try_to_freeze()
requirement in the kthread documentation... Unfortunately I cant find
any kthread docs in Documentation/ :)
Anton
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
2007-03-01 19:38 ` Anton Blanchard
@ 2007-03-01 19:54 ` Rafael J. Wysocki
2007-03-02 21:35 ` Paul E. McKenney
0 siblings, 1 reply; 46+ messages in thread
From: Rafael J. Wysocki @ 2007-03-01 19:54 UTC (permalink / raw)
To: Anton Blanchard, Paul E. McKenney
Cc: Andrew Morton, Pavel Machek, LKML, Oleg Nesterov, Aneesh Kumar,
Srivatsa Vaddagiri, Gautham R Shenoy
On Thursday, 1 March 2007 20:38, Anton Blanchard wrote:
>
> Hi,
>
> > Remove PF_NOFREEZE from the rcutorture thread, adding a
> > try_to_freeze() call as required.
>
> ...
>
> > @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
> > }
> > rcu_torture_current_version++;
> > oldbatch = cur_ops->completed();
> > + try_to_freeze();
> > } while (!kthread_should_stop() && !fullstop);
> > VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> > while (!kthread_should_stop())
>
> I wonder if it makes sense to embed try_to_freeze() into the kthread
> API somewhere. Short of that we should document the try_to_freeze()
> requirement in the kthread documentation... Unfortunately I cant find
> any kthread docs in Documentation/ :)
Well, the patch is from Paul, so I think he'll be able to comment. :-)
Greetings,
Rafael
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
2007-03-01 19:54 ` Rafael J. Wysocki
@ 2007-03-02 21:35 ` Paul E. McKenney
2007-03-02 22:16 ` Anton Blanchard
2007-03-02 23:33 ` Oleg Nesterov
0 siblings, 2 replies; 46+ messages in thread
From: Paul E. McKenney @ 2007-03-02 21:35 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Anton Blanchard, Andrew Morton, Pavel Machek, LKML,
Oleg Nesterov, Aneesh Kumar, Srivatsa Vaddagiri,
Gautham R Shenoy
On Thu, Mar 01, 2007 at 08:54:25PM +0100, Rafael J. Wysocki wrote:
> On Thursday, 1 March 2007 20:38, Anton Blanchard wrote:
> >
> > Hi,
> >
> > > Remove PF_NOFREEZE from the rcutorture thread, adding a
> > > try_to_freeze() call as required.
> >
> > ...
> >
> > > @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
> > > }
> > > rcu_torture_current_version++;
> > > oldbatch = cur_ops->completed();
> > > + try_to_freeze();
> > > } while (!kthread_should_stop() && !fullstop);
> > > VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> > > while (!kthread_should_stop())
> >
> > I wonder if it makes sense to embed try_to_freeze() into the kthread
> > API somewhere. Short of that we should document the try_to_freeze()
> > requirement in the kthread documentation... Unfortunately I cant find
> > any kthread docs in Documentation/ :)
>
> Well, the patch is from Paul, so I think he'll be able to comment. :-)
We certainly either need to embed try_to_freeze() into kthread_should_stop()
or add back the rcu_torture_fakewriter(), and rcu_torture_reader()
components of this patch. ;-)
One way to embed try_to_freeze() into kthread_should_stop() might be
as follows:
int kthread_should_stop(void)
{
if (kthread_stop_info.k == current)
return 1;
try_to_freeze();
return 0;
}
Does this seem reasonable? It certainly would cut down some of the
code required for freezing -- and reduce the potential for bugs.
Thanx, Paul
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
2007-03-02 21:35 ` Paul E. McKenney
@ 2007-03-02 22:16 ` Anton Blanchard
2007-03-02 23:33 ` Oleg Nesterov
1 sibling, 0 replies; 46+ messages in thread
From: Anton Blanchard @ 2007-03-02 22:16 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Rafael J. Wysocki, Andrew Morton, Pavel Machek, LKML,
Oleg Nesterov, Aneesh Kumar, Srivatsa Vaddagiri,
Gautham R Shenoy
Hi Paul,
> We certainly either need to embed try_to_freeze() into kthread_should_stop()
> or add back the rcu_torture_fakewriter(), and rcu_torture_reader()
> components of this patch. ;-)
>
> One way to embed try_to_freeze() into kthread_should_stop() might be
> as follows:
>
> int kthread_should_stop(void)
> {
> if (kthread_stop_info.k == current)
> return 1;
> try_to_freeze();
> return 0;
> }
>
> Does this seem reasonable? It certainly would cut down some of the
> code required for freezing -- and reduce the potential for bugs.
Thats what I was thinking too :)
Anton
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
2007-03-02 21:35 ` Paul E. McKenney
2007-03-02 22:16 ` Anton Blanchard
@ 2007-03-02 23:33 ` Oleg Nesterov
2007-03-03 0:58 ` Paul E. McKenney
2007-03-03 1:03 ` [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread Rafael J. Wysocki
1 sibling, 2 replies; 46+ messages in thread
From: Oleg Nesterov @ 2007-03-02 23:33 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Rafael J. Wysocki, Anton Blanchard, Andrew Morton, Pavel Machek,
LKML, Aneesh Kumar, Srivatsa Vaddagiri, Gautham R Shenoy
On 03/02, Paul E. McKenney wrote:
>
> One way to embed try_to_freeze() into kthread_should_stop() might be
> as follows:
>
> int kthread_should_stop(void)
> {
> if (kthread_stop_info.k == current)
> return 1;
> try_to_freeze();
> return 0;
> }
I think this is dangerous. For example, worker_thread() will probably
need some special actions after return from refrigerator. Also, a kernel
thread may check kthread_should_stop() in the place where try_to_freeze()
is not safe.
Perhaps we should introduce a new helper which does this.
Oleg.
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
2007-03-02 23:33 ` Oleg Nesterov
@ 2007-03-03 0:58 ` Paul E. McKenney
2007-03-03 17:32 ` Oleg Nesterov
2007-03-03 1:03 ` [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread Rafael J. Wysocki
1 sibling, 1 reply; 46+ messages in thread
From: Paul E. McKenney @ 2007-03-03 0:58 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Rafael J. Wysocki, Anton Blanchard, Andrew Morton, Pavel Machek,
LKML, Aneesh Kumar, Srivatsa Vaddagiri, Gautham R Shenoy
On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> On 03/02, Paul E. McKenney wrote:
> >
> > One way to embed try_to_freeze() into kthread_should_stop() might be
> > as follows:
> >
> > int kthread_should_stop(void)
> > {
> > if (kthread_stop_info.k == current)
> > return 1;
> > try_to_freeze();
> > return 0;
> > }
>
> I think this is dangerous. For example, worker_thread() will probably
> need some special actions after return from refrigerator. Also, a kernel
> thread may check kthread_should_stop() in the place where try_to_freeze()
> is not safe.
>
> Perhaps we should introduce a new helper which does this.
Good point -- the return value from try_to_freeze() is lost if one uses
the above approach. About one third of the calls to try_to_freeze()
in 2.6.20 pay attention to the return value.
One approach would be to have a kthread_should_stop_nofreeze() for those
cases, and let the default be to try to freeze.
Is this what you had in mind?
Thanx, Paul
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
2007-03-03 0:58 ` Paul E. McKenney
@ 2007-03-03 17:32 ` Oleg Nesterov
2007-03-04 4:34 ` Srivatsa Vaddagiri
2007-03-11 17:49 ` [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread) Rafael J. Wysocki
0 siblings, 2 replies; 46+ messages in thread
From: Oleg Nesterov @ 2007-03-03 17:32 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Rafael J. Wysocki, Anton Blanchard, Andrew Morton, Pavel Machek,
LKML, Aneesh Kumar, Srivatsa Vaddagiri, Gautham R Shenoy
On 03/02, Paul E. McKenney wrote:
>
> On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> > On 03/02, Paul E. McKenney wrote:
> > >
> > > One way to embed try_to_freeze() into kthread_should_stop() might be
> > > as follows:
> > >
> > > int kthread_should_stop(void)
> > > {
> > > if (kthread_stop_info.k == current)
> > > return 1;
> > > try_to_freeze();
> > > return 0;
> > > }
> >
> > I think this is dangerous. For example, worker_thread() will probably
> > need some special actions after return from refrigerator. Also, a kernel
> > thread may check kthread_should_stop() in the place where try_to_freeze()
> > is not safe.
> >
> > Perhaps we should introduce a new helper which does this.
>
> Good point -- the return value from try_to_freeze() is lost if one uses
> the above approach. About one third of the calls to try_to_freeze()
> in 2.6.20 pay attention to the return value.
>
> One approach would be to have a kthread_should_stop_nofreeze() for those
> cases, and let the default be to try to freeze.
I personally think we should do the opposite, add kthread_should_stop_check_freeze()
or something. kthread_should_stop() is like signal_pending(), we can use
it under spin_lock (and it is probably used this way by some out-of-tree
driver). The new helper is obviously "might_sleep()".
Oleg.
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
2007-03-03 17:32 ` Oleg Nesterov
@ 2007-03-04 4:34 ` Srivatsa Vaddagiri
2007-03-11 17:49 ` [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread) Rafael J. Wysocki
1 sibling, 0 replies; 46+ messages in thread
From: Srivatsa Vaddagiri @ 2007-03-04 4:34 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Paul E. McKenney, Rafael J. Wysocki, Anton Blanchard,
Andrew Morton, Pavel Machek, LKML, Aneesh Kumar,
Gautham R Shenoy
On Sat, Mar 03, 2007 at 08:32:40PM +0300, Oleg Nesterov wrote:
> I personally think we should do the opposite, add kthread_should_stop_check_freeze()
> or something. kthread_should_stop() is like signal_pending(), we can use
> it under spin_lock (and it is probably used this way by some out-of-tree
> driver). The new helper is obviously "might_sleep()".
Yes, I agree. It helps the caller explicitly know that this function
-can- freeze.
--
Regards,
vatsa
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-03 17:32 ` Oleg Nesterov
2007-03-04 4:34 ` Srivatsa Vaddagiri
@ 2007-03-11 17:49 ` Rafael J. Wysocki
2007-03-12 4:38 ` Paul E. McKenney
2007-03-13 5:27 ` Gautham R Shenoy
1 sibling, 2 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2007-03-11 17:49 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Paul E. McKenney, Anton Blanchard, Andrew Morton, Pavel Machek,
LKML, Aneesh Kumar, Srivatsa Vaddagiri, Gautham R Shenoy
On Saturday, 3 March 2007 18:32, Oleg Nesterov wrote:
> On 03/02, Paul E. McKenney wrote:
> >
> > On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> > > On 03/02, Paul E. McKenney wrote:
> > > >
> > > > One way to embed try_to_freeze() into kthread_should_stop() might be
> > > > as follows:
> > > >
> > > > int kthread_should_stop(void)
> > > > {
> > > > if (kthread_stop_info.k == current)
> > > > return 1;
> > > > try_to_freeze();
> > > > return 0;
> > > > }
> > >
> > > I think this is dangerous. For example, worker_thread() will probably
> > > need some special actions after return from refrigerator. Also, a kernel
> > > thread may check kthread_should_stop() in the place where try_to_freeze()
> > > is not safe.
> > >
> > > Perhaps we should introduce a new helper which does this.
> >
> > Good point -- the return value from try_to_freeze() is lost if one uses
> > the above approach. About one third of the calls to try_to_freeze()
> > in 2.6.20 pay attention to the return value.
> >
> > One approach would be to have a kthread_should_stop_nofreeze() for those
> > cases, and let the default be to try to freeze.
>
> I personally think we should do the opposite, add kthread_should_stop_check_freeze()
> or something. kthread_should_stop() is like signal_pending(), we can use
> it under spin_lock (and it is probably used this way by some out-of-tree
> driver). The new helper is obviously "might_sleep()".
Something like this, perhaps:
include/linux/kthread.h | 1 +
kernel/kthread.c | 16 ++++++++++++++++
kernel/rcutorture.c | 5 ++---
3 files changed, 19 insertions(+), 3 deletions(-)
Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
===================================================================
--- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c 2007-03-08 21:58:48.000000000 +0100
+++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.000000000 +0100
@@ -13,6 +13,7 @@
#include <linux/file.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/freezer.h>
#include <asm/semaphore.h>
/*
@@ -60,6 +61,21 @@ int kthread_should_stop(void)
}
EXPORT_SYMBOL(kthread_should_stop);
+/**
+ * kthread_should_stop_check_freeze - check if the thread should return now and
+ * if not, check if there is a freezing request pending for it.
+ */
+int kthread_should_stop_check_freeze(void)
+{
+ might_sleep();
+ if (kthread_stop_info.k == current)
+ return 1;
+
+ try_to_freeze();
+ return 0;
+}
+EXPORT_SYMBOL(kthread_should_stop_check_freeze);
+
static void kthread_exit_files(void)
{
struct fs_struct *fs;
Index: linux-2.6.21-rc3-mm2/include/linux/kthread.h
===================================================================
--- linux-2.6.21-rc3-mm2.orig/include/linux/kthread.h 2007-02-04 19:44:54.000000000 +0100
+++ linux-2.6.21-rc3-mm2/include/linux/kthread.h 2007-03-11 18:37:10.000000000 +0100
@@ -29,5 +29,6 @@ struct task_struct *kthread_create(int (
void kthread_bind(struct task_struct *k, unsigned int cpu);
int kthread_stop(struct task_struct *k);
int kthread_should_stop(void);
+int kthread_should_stop_check_freeze(void);
#endif /* _LINUX_KTHREAD_H */
Index: linux-2.6.21-rc3-mm2/kernel/rcutorture.c
===================================================================
--- linux-2.6.21-rc3-mm2.orig/kernel/rcutorture.c 2007-03-11 11:39:06.000000000 +0100
+++ linux-2.6.21-rc3-mm2/kernel/rcutorture.c 2007-03-11 18:45:00.000000000 +0100
@@ -540,10 +540,9 @@ rcu_torture_writer(void *arg)
}
rcu_torture_current_version++;
oldbatch = cur_ops->completed();
- try_to_freeze();
- } while (!kthread_should_stop() && !fullstop);
+ } while (!kthread_should_stop_check_freeze() && !fullstop);
VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
- while (!kthread_should_stop())
+ while (!kthread_should_stop_check_freeze())
schedule_timeout_uninterruptible(1);
return 0;
}
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-11 17:49 ` [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread) Rafael J. Wysocki
@ 2007-03-12 4:38 ` Paul E. McKenney
2007-03-12 8:14 ` Pavel Machek
2007-03-13 5:27 ` Gautham R Shenoy
1 sibling, 1 reply; 46+ messages in thread
From: Paul E. McKenney @ 2007-03-12 4:38 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Oleg Nesterov, Anton Blanchard, Andrew Morton, Pavel Machek,
LKML, Aneesh Kumar, Srivatsa Vaddagiri, Gautham R Shenoy
On Sun, Mar 11, 2007 at 06:49:08PM +0100, Rafael J. Wysocki wrote:
> On Saturday, 3 March 2007 18:32, Oleg Nesterov wrote:
> > On 03/02, Paul E. McKenney wrote:
> > >
> > > On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> > > > On 03/02, Paul E. McKenney wrote:
> > > > >
> > > > > One way to embed try_to_freeze() into kthread_should_stop() might be
> > > > > as follows:
> > > > >
> > > > > int kthread_should_stop(void)
> > > > > {
> > > > > if (kthread_stop_info.k == current)
> > > > > return 1;
> > > > > try_to_freeze();
> > > > > return 0;
> > > > > }
> > > >
> > > > I think this is dangerous. For example, worker_thread() will probably
> > > > need some special actions after return from refrigerator. Also, a kernel
> > > > thread may check kthread_should_stop() in the place where try_to_freeze()
> > > > is not safe.
> > > >
> > > > Perhaps we should introduce a new helper which does this.
> > >
> > > Good point -- the return value from try_to_freeze() is lost if one uses
> > > the above approach. About one third of the calls to try_to_freeze()
> > > in 2.6.20 pay attention to the return value.
> > >
> > > One approach would be to have a kthread_should_stop_nofreeze() for those
> > > cases, and let the default be to try to freeze.
> >
> > I personally think we should do the opposite, add kthread_should_stop_check_freeze()
> > or something. kthread_should_stop() is like signal_pending(), we can use
> > it under spin_lock (and it is probably used this way by some out-of-tree
> > driver). The new helper is obviously "might_sleep()".
>
> Something like this, perhaps:
Looks good to me! The other kthread_should_stop() calls in
rcutorture.c should also become kthread_should_top_check_freeze().
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> include/linux/kthread.h | 1 +
> kernel/kthread.c | 16 ++++++++++++++++
> kernel/rcutorture.c | 5 ++---
> 3 files changed, 19 insertions(+), 3 deletions(-)
>
> Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
> ===================================================================
> --- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c 2007-03-08 21:58:48.000000000 +0100
> +++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.000000000 +0100
> @@ -13,6 +13,7 @@
> #include <linux/file.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> +#include <linux/freezer.h>
> #include <asm/semaphore.h>
>
> /*
> @@ -60,6 +61,21 @@ int kthread_should_stop(void)
> }
> EXPORT_SYMBOL(kthread_should_stop);
>
> +/**
> + * kthread_should_stop_check_freeze - check if the thread should return now and
> + * if not, check if there is a freezing request pending for it.
> + */
> +int kthread_should_stop_check_freeze(void)
> +{
> + might_sleep();
> + if (kthread_stop_info.k == current)
> + return 1;
> +
> + try_to_freeze();
> + return 0;
> +}
> +EXPORT_SYMBOL(kthread_should_stop_check_freeze);
> +
> static void kthread_exit_files(void)
> {
> struct fs_struct *fs;
> Index: linux-2.6.21-rc3-mm2/include/linux/kthread.h
> ===================================================================
> --- linux-2.6.21-rc3-mm2.orig/include/linux/kthread.h 2007-02-04 19:44:54.000000000 +0100
> +++ linux-2.6.21-rc3-mm2/include/linux/kthread.h 2007-03-11 18:37:10.000000000 +0100
> @@ -29,5 +29,6 @@ struct task_struct *kthread_create(int (
> void kthread_bind(struct task_struct *k, unsigned int cpu);
> int kthread_stop(struct task_struct *k);
> int kthread_should_stop(void);
> +int kthread_should_stop_check_freeze(void);
>
> #endif /* _LINUX_KTHREAD_H */
> Index: linux-2.6.21-rc3-mm2/kernel/rcutorture.c
> ===================================================================
> --- linux-2.6.21-rc3-mm2.orig/kernel/rcutorture.c 2007-03-11 11:39:06.000000000 +0100
> +++ linux-2.6.21-rc3-mm2/kernel/rcutorture.c 2007-03-11 18:45:00.000000000 +0100
> @@ -540,10 +540,9 @@ rcu_torture_writer(void *arg)
> }
> rcu_torture_current_version++;
> oldbatch = cur_ops->completed();
> - try_to_freeze();
> - } while (!kthread_should_stop() && !fullstop);
> + } while (!kthread_should_stop_check_freeze() && !fullstop);
> VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> - while (!kthread_should_stop())
> + while (!kthread_should_stop_check_freeze())
> schedule_timeout_uninterruptible(1);
> return 0;
> }
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-12 4:38 ` Paul E. McKenney
@ 2007-03-12 8:14 ` Pavel Machek
2007-03-12 11:19 ` Rafael J. Wysocki
0 siblings, 1 reply; 46+ messages in thread
From: Pavel Machek @ 2007-03-12 8:14 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Rafael J. Wysocki, Oleg Nesterov, Anton Blanchard, Andrew Morton,
LKML, Aneesh Kumar, Srivatsa Vaddagiri, Gautham R Shenoy
Hi!
> > > I personally think we should do the opposite, add kthread_should_stop_check_freeze()
> > > or something. kthread_should_stop() is like signal_pending(), we can use
> > > it under spin_lock (and it is probably used this way by some out-of-tree
> > > driver). The new helper is obviously "might_sleep()".
> >
> > Something like this, perhaps:
>
> Looks good to me! The other kthread_should_stop() calls in
> rcutorture.c should also become kthread_should_top_check_freeze().
>
> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>
> > include/linux/kthread.h | 1 +
> > kernel/kthread.c | 16 ++++++++++++++++
> > kernel/rcutorture.c | 5 ++---
> > 3 files changed, 19 insertions(+), 3 deletions(-)
> >
> > Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
> > ===================================================================
> > --- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c 2007-03-08 21:58:48.000000000 +0100
> > +++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.000000000 +0100
> > @@ -13,6 +13,7 @@
> > #include <linux/file.h>
> > #include <linux/module.h>
> > #include <linux/mutex.h>
> > +#include <linux/freezer.h>
> > #include <asm/semaphore.h>
> >
> > /*
> > @@ -60,6 +61,21 @@ int kthread_should_stop(void)
> > }
> > EXPORT_SYMBOL(kthread_should_stop);
> >
> > +/**
> > + * kthread_should_stop_check_freeze - check if the thread should return now and
> > + * if not, check if there is a freezing request pending for it.
> > + */
> > +int kthread_should_stop_check_freeze(void)
> > +{
> > + might_sleep();
> > + if (kthread_stop_info.k == current)
> > + return 1;
> > +
> > + try_to_freeze();
> > + return 0;
> > +}
Can we get better name for this function?
Why is it useful? Caller can do "try_to_freeze()" as well, no?
> > }
> > rcu_torture_current_version++;
> > oldbatch = cur_ops->completed();
> > - try_to_freeze();
> > - } while (!kthread_should_stop() && !fullstop);
> > + } while (!kthread_should_stop_check_freeze() && !fullstop);
> > VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> > - while (!kthread_should_stop())
> > + while (!kthread_should_stop_check_freeze())
> > schedule_timeout_uninterruptible(1);
> > return 0;
Aha, I see, here it probably becomes handy.
Actually, no... I do not see it. Why don't you simply move first
try_to_freeze() to beggining of the loop and do
- while (!kthread_should_stop()) {
schedule_timeout_uninterruptible(1);
try_to_freeze()
}
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-12 8:14 ` Pavel Machek
@ 2007-03-12 11:19 ` Rafael J. Wysocki
2007-03-12 12:23 ` Oleg Nesterov
2007-03-12 22:39 ` [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread) Pavel Machek
0 siblings, 2 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2007-03-12 11:19 UTC (permalink / raw)
To: Pavel Machek
Cc: Paul E. McKenney, Oleg Nesterov, Anton Blanchard, Andrew Morton,
LKML, Aneesh Kumar, Srivatsa Vaddagiri, Gautham R Shenoy
Hi,
On Monday, 12 March 2007 09:14, Pavel Machek wrote:
> Hi!
>
> > > > I personally think we should do the opposite, add kthread_should_stop_check_freeze()
> > > > or something. kthread_should_stop() is like signal_pending(), we can use
> > > > it under spin_lock (and it is probably used this way by some out-of-tree
> > > > driver). The new helper is obviously "might_sleep()".
> > >
> > > Something like this, perhaps:
> >
> > Looks good to me! The other kthread_should_stop() calls in
> > rcutorture.c should also become kthread_should_top_check_freeze().
> >
> > Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> >
> > > include/linux/kthread.h | 1 +
> > > kernel/kthread.c | 16 ++++++++++++++++
> > > kernel/rcutorture.c | 5 ++---
> > > 3 files changed, 19 insertions(+), 3 deletions(-)
> > >
> > > Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
> > > ===================================================================
> > > --- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c 2007-03-08 21:58:48.000000000 +0100
> > > +++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.000000000 +0100
> > > @@ -13,6 +13,7 @@
> > > #include <linux/file.h>
> > > #include <linux/module.h>
> > > #include <linux/mutex.h>
> > > +#include <linux/freezer.h>
> > > #include <asm/semaphore.h>
> > >
> > > /*
> > > @@ -60,6 +61,21 @@ int kthread_should_stop(void)
> > > }
> > > EXPORT_SYMBOL(kthread_should_stop);
> > >
> > > +/**
> > > + * kthread_should_stop_check_freeze - check if the thread should return now and
> > > + * if not, check if there is a freezing request pending for it.
> > > + */
> > > +int kthread_should_stop_check_freeze(void)
> > > +{
> > > + might_sleep();
> > > + if (kthread_stop_info.k == current)
> > > + return 1;
> > > +
> > > + try_to_freeze();
> > > + return 0;
> > > +}
>
> Can we get better name for this function?
Well, I took the name from the Oleg's message. Can you please suggest
something?
> Why is it useful?
Because we want to avoid repeating
while (!kthread_should_stop()) {
try_to_freeze();
...
}
in many places?
> Caller can do "try_to_freeze()" as well, no?
Sure, it just eliminates one line of code.
> > > }
> > > rcu_torture_current_version++;
> > > oldbatch = cur_ops->completed();
> > > - try_to_freeze();
> > > - } while (!kthread_should_stop() && !fullstop);
> > > + } while (!kthread_should_stop_check_freeze() && !fullstop);
> > > VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> > > - while (!kthread_should_stop())
> > > + while (!kthread_should_stop_check_freeze())
> > > schedule_timeout_uninterruptible(1);
> > > return 0;
>
> Aha, I see, here it probably becomes handy.
>
> Actually, no... I do not see it. Why don't you simply move first
> try_to_freeze() to beggining of the loop and do
>
> - while (!kthread_should_stop()) {
> schedule_timeout_uninterruptible(1);
> try_to_freeze()
> }
Yes, but then the second loop will contain one more line of code.
Greetings,
Rafael
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-12 11:19 ` Rafael J. Wysocki
@ 2007-03-12 12:23 ` Oleg Nesterov
2007-03-12 13:24 ` [PATCH] kthread_should_stop_check_freeze Cedric Le Goater
2007-03-12 22:39 ` [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread) Pavel Machek
1 sibling, 1 reply; 46+ messages in thread
From: Oleg Nesterov @ 2007-03-12 12:23 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Pavel Machek, Paul E. McKenney, Anton Blanchard, Andrew Morton,
LKML, Aneesh Kumar, Srivatsa Vaddagiri, Gautham R Shenoy
On 03/12, Rafael J. Wysocki wrote:
>
> On Monday, 12 March 2007 09:14, Pavel Machek wrote:
> >
> > Can we get better name for this function?
>
> Well, I took the name from the Oleg's message. Can you please suggest
> something?
Well, kthread_should_stop_check_freeze() is really awful, I agree :)
We need something better, but I can't suggest anything.
Oleg.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] kthread_should_stop_check_freeze
2007-03-12 12:23 ` Oleg Nesterov
@ 2007-03-12 13:24 ` Cedric Le Goater
2007-03-12 18:25 ` Rafael J. Wysocki
0 siblings, 1 reply; 46+ messages in thread
From: Cedric Le Goater @ 2007-03-12 13:24 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Rafael J. Wysocki, Pavel Machek, Paul E. McKenney,
Anton Blanchard, Andrew Morton, LKML, Aneesh Kumar,
Srivatsa Vaddagiri, Gautham R Shenoy
Oleg Nesterov wrote:
> On 03/12, Rafael J. Wysocki wrote:
>> On Monday, 12 March 2007 09:14, Pavel Machek wrote:
>>> Can we get better name for this function?
>> Well, I took the name from the Oleg's message. Can you please suggest
>> something?
>
> Well, kthread_should_stop_check_freeze() is really awful, I agree :)
> We need something better, but I can't suggest anything.
not much better, but what about kthread_should_stop_or_freeze() ?
cheers,
C.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] kthread_should_stop_check_freeze
2007-03-12 13:24 ` [PATCH] kthread_should_stop_check_freeze Cedric Le Goater
@ 2007-03-12 18:25 ` Rafael J. Wysocki
0 siblings, 0 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2007-03-12 18:25 UTC (permalink / raw)
To: Cedric Le Goater
Cc: Oleg Nesterov, Pavel Machek, Paul E. McKenney, Anton Blanchard,
Andrew Morton, LKML, Aneesh Kumar, Srivatsa Vaddagiri,
Gautham R Shenoy
On Monday, 12 March 2007 14:24, Cedric Le Goater wrote:
> Oleg Nesterov wrote:
> > On 03/12, Rafael J. Wysocki wrote:
> >> On Monday, 12 March 2007 09:14, Pavel Machek wrote:
> >>> Can we get better name for this function?
> >> Well, I took the name from the Oleg's message. Can you please suggest
> >> something?
> >
> > Well, kthread_should_stop_check_freeze() is really awful, I agree :)
> > We need something better, but I can't suggest anything.
>
> not much better, but what about kthread_should_stop_or_freeze() ?
IMO this suggests that the thread will either stop or freeze itself, but the
freezing part is not unconditional.
I was thinking of kthread_should_stop_freezable() .
Greetings,
Rafael
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-12 11:19 ` Rafael J. Wysocki
2007-03-12 12:23 ` Oleg Nesterov
@ 2007-03-12 22:39 ` Pavel Machek
2007-03-12 22:45 ` Anton Blanchard
2007-03-13 0:58 ` Paul E. McKenney
1 sibling, 2 replies; 46+ messages in thread
From: Pavel Machek @ 2007-03-12 22:39 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Paul E. McKenney, Oleg Nesterov, Anton Blanchard, Andrew Morton,
LKML, Aneesh Kumar, Srivatsa Vaddagiri, Gautham R Shenoy
Hi!
> > > Looks good to me! The other kthread_should_stop() calls in
> > > rcutorture.c should also become
> > > kthread_should_top_check_freeze().
> > Why is it useful?
>
> Because we want to avoid repeating
>
> while (!kthread_should_stop()) {
> try_to_freeze();
> ...
> }
>
> in many places?
Do not do it, then. Confusion it causes is not worth saving one line
of code.
You do less typing, but the resulting code is _less_ readable, not
more.
NAK.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-12 22:39 ` [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread) Pavel Machek
@ 2007-03-12 22:45 ` Anton Blanchard
2007-03-13 3:14 ` Srivatsa Vaddagiri
2007-03-20 18:08 ` Pavel Machek
2007-03-13 0:58 ` Paul E. McKenney
1 sibling, 2 replies; 46+ messages in thread
From: Anton Blanchard @ 2007-03-12 22:45 UTC (permalink / raw)
To: Pavel Machek
Cc: Rafael J. Wysocki, Paul E. McKenney, Oleg Nesterov,
Andrew Morton, LKML, Aneesh Kumar, Srivatsa Vaddagiri,
Gautham R Shenoy
> Do not do it, then. Confusion it causes is not worth saving one line
> of code.
>
> You do less typing, but the resulting code is _less_ readable, not
> more.
Then please document it _clearly_ with the kthread code somewhere. The
reason I brought this up is I had no idea we had to put the freezer gunk
in all kernel thread loops and Ive been writing kernel threads for years.
Anton
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-12 22:45 ` Anton Blanchard
@ 2007-03-13 3:14 ` Srivatsa Vaddagiri
2007-03-13 9:16 ` Christoph Hellwig
2007-03-20 18:08 ` Pavel Machek
1 sibling, 1 reply; 46+ messages in thread
From: Srivatsa Vaddagiri @ 2007-03-13 3:14 UTC (permalink / raw)
To: Anton Blanchard
Cc: Pavel Machek, Rafael J. Wysocki, Paul E. McKenney, Oleg Nesterov,
Andrew Morton, LKML, Aneesh Kumar, Gautham R Shenoy
On Mon, Mar 12, 2007 at 05:45:24PM -0500, Anton Blanchard wrote:
> Then please document it _clearly_ with the kthread code somewhere.
Document as well in the kernel_thread() API, as I notice people still
use kernel_thread() some places (ex: rtasd.c in powerpc arch)?
> The reason I brought this up is I had no idea we had to put the freezer gunk
> in all kernel thread loops and Ive been writing kernel threads for years.
I noticed that in the Powerpc code (atleast for rtas kernel thread)
here:
http://lkml.org/lkml/2007/1/9/61
That was not a serious problem perhaps because process freezer was mostly used
in software suspend and only those platforms supporting software suspend
had to worry abt it.
But now we intend to use process freezer for CPU hotplug as well, so all
platforms wanting to support CPU hotplug better support process freezer!
P.S : I believe kprobes is already using process freezer as well.
--
Regards,
vatsa
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-13 3:14 ` Srivatsa Vaddagiri
@ 2007-03-13 9:16 ` Christoph Hellwig
2007-03-13 9:58 ` Srivatsa Vaddagiri
0 siblings, 1 reply; 46+ messages in thread
From: Christoph Hellwig @ 2007-03-13 9:16 UTC (permalink / raw)
To: Srivatsa Vaddagiri
Cc: Anton Blanchard, Pavel Machek, Rafael J. Wysocki,
Paul E. McKenney, Oleg Nesterov, Andrew Morton, LKML,
Aneesh Kumar, Gautham R Shenoy
On Tue, Mar 13, 2007 at 08:44:11AM +0530, Srivatsa Vaddagiri wrote:
> On Mon, Mar 12, 2007 at 05:45:24PM -0500, Anton Blanchard wrote:
> > Then please document it _clearly_ with the kthread code somewhere.
>
> Document as well in the kernel_thread() API, as I notice people still
> use kernel_thread() some places (ex: rtasd.c in powerpc arch)?
They shouldn't use kernel_thread.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-13 9:16 ` Christoph Hellwig
@ 2007-03-13 9:58 ` Srivatsa Vaddagiri
2007-03-13 10:20 ` Christoph Hellwig
0 siblings, 1 reply; 46+ messages in thread
From: Srivatsa Vaddagiri @ 2007-03-13 9:58 UTC (permalink / raw)
To: Christoph Hellwig, Anton Blanchard, Pavel Machek,
Rafael J. Wysocki, Paul E. McKenney, Oleg Nesterov,
Andrew Morton, LKML, Aneesh Kumar, Gautham R Shenoy
On Tue, Mar 13, 2007 at 09:16:29AM +0000, Christoph Hellwig wrote:
> > Document as well in the kernel_thread() API, as I notice people still
> > use kernel_thread() some places (ex: rtasd.c in powerpc arch)?
>
> They shouldn't use kernel_thread.
Hmm ..that needs to be documented as well then! I can easily count more
than dozen places where kernel_thread() is being used.
I agree though that kthread is a much cleaner interface to create/destroy
threads.
--
Regards,
vatsa
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-13 9:58 ` Srivatsa Vaddagiri
@ 2007-03-13 10:20 ` Christoph Hellwig
0 siblings, 0 replies; 46+ messages in thread
From: Christoph Hellwig @ 2007-03-13 10:20 UTC (permalink / raw)
To: Srivatsa Vaddagiri
Cc: Christoph Hellwig, Anton Blanchard, Pavel Machek,
Rafael J. Wysocki, Paul E. McKenney, Oleg Nesterov,
Andrew Morton, LKML, Aneesh Kumar, Gautham R Shenoy
On Tue, Mar 13, 2007 at 03:28:08PM +0530, Srivatsa Vaddagiri wrote:
> On Tue, Mar 13, 2007 at 09:16:29AM +0000, Christoph Hellwig wrote:
> > > Document as well in the kernel_thread() API, as I notice people still
> > > use kernel_thread() some places (ex: rtasd.c in powerpc arch)?
> >
> > They shouldn't use kernel_thread.
>
> Hmm ..that needs to be documented as well then! I can easily count more
> than dozen places where kernel_thread() is being used.
>
> I agree though that kthread is a much cleaner interface to create/destroy
> threads.
Well, it takes a lot of time to convert all the existing users. But
I try to make sure to flame^H^H^H^H^Hcorrect everyone who tries to sneak
a new user of kernel_thread in.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-12 22:45 ` Anton Blanchard
2007-03-13 3:14 ` Srivatsa Vaddagiri
@ 2007-03-20 18:08 ` Pavel Machek
1 sibling, 0 replies; 46+ messages in thread
From: Pavel Machek @ 2007-03-20 18:08 UTC (permalink / raw)
To: Anton Blanchard
Cc: Rafael J. Wysocki, Paul E. McKenney, Oleg Nesterov,
Andrew Morton, LKML, Aneesh Kumar, Srivatsa Vaddagiri,
Gautham R Shenoy
Hi!
> > Do not do it, then. Confusion it causes is not worth saving one line
> > of code.
> >
> > You do less typing, but the resulting code is _less_ readable, not
> > more.
>
> Then please document it _clearly_ with the kthread code somewhere. The
> reason I brought this up is I had no idea we had to put the freezer gunk
> in all kernel thread loops and Ive been writing kernel threads for years.
This is in Doc*/power/kernel_threads.txt:
KERNEL THREADS
Freezer
Upon entering a suspended state the system will freeze all
tasks. This is done by delivering pseudosignals. This affects
kernel threads, too. To successfully freeze a kernel thread
the thread has to check for the pseudosignal and enter the
refrigerator. Code to do this looks like this:
do {
hub_events();
wait_event_interruptible(khubd_wait,
!list_empty(&hub_event_list));
try_to_freeze();
} while (!signal_pending(current));
...do you know about better place?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-12 22:39 ` [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread) Pavel Machek
2007-03-12 22:45 ` Anton Blanchard
@ 2007-03-13 0:58 ` Paul E. McKenney
2007-03-20 18:10 ` Pavel Machek
1 sibling, 1 reply; 46+ messages in thread
From: Paul E. McKenney @ 2007-03-13 0:58 UTC (permalink / raw)
To: Pavel Machek
Cc: Rafael J. Wysocki, Oleg Nesterov, Anton Blanchard, Andrew Morton,
LKML, Aneesh Kumar, Srivatsa Vaddagiri, Gautham R Shenoy
On Mon, Mar 12, 2007 at 11:39:06PM +0100, Pavel Machek wrote:
> Hi!
>
> > > > Looks good to me! The other kthread_should_stop() calls in
> > > > rcutorture.c should also become
> > > > kthread_should_top_check_freeze().
>
> > > Why is it useful?
> >
> > Because we want to avoid repeating
> >
> > while (!kthread_should_stop()) {
> > try_to_freeze();
> > ...
> > }
> >
> > in many places?
>
> Do not do it, then. Confusion it causes is not worth saving one line
> of code.
>
> You do less typing, but the resulting code is _less_ readable, not
> more.
>
> NAK.
Another problem is people doing "kthread_should_stop()" and forgetting
about freezing. Then we continue ending up with situations where we are
intermittently unable to freeze. In the spirit of "Rusty Scale" interface
design, how do we make it difficult for people to misuse this interface?
Thanx, Paul
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-13 0:58 ` Paul E. McKenney
@ 2007-03-20 18:10 ` Pavel Machek
0 siblings, 0 replies; 46+ messages in thread
From: Pavel Machek @ 2007-03-20 18:10 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Rafael J. Wysocki, Oleg Nesterov, Anton Blanchard, Andrew Morton,
LKML, Aneesh Kumar, Srivatsa Vaddagiri, Gautham R Shenoy
On Mon 2007-03-12 17:58:05, Paul E. McKenney wrote:
> On Mon, Mar 12, 2007 at 11:39:06PM +0100, Pavel Machek wrote:
> > Hi!
> >
> > > > > Looks good to me! The other kthread_should_stop() calls in
> > > > > rcutorture.c should also become
> > > > > kthread_should_top_check_freeze().
> >
> > > > Why is it useful?
> > >
> > > Because we want to avoid repeating
> > >
> > > while (!kthread_should_stop()) {
> > > try_to_freeze();
> > > ...
> > > }
> > >
> > > in many places?
> >
> > Do not do it, then. Confusion it causes is not worth saving one line
> > of code.
> >
> > You do less typing, but the resulting code is _less_ readable, not
> > more.
> >
> > NAK.
>
> Another problem is people doing "kthread_should_stop()" and forgetting
> about freezing. Then we continue ending up with situations where weare
> intermittently unable to freeze. In the spirit of "Rusty Scale" interface
No, in such case we end up with situation when it is impossible to
freeze, and that's very easy to debug.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-11 17:49 ` [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread) Rafael J. Wysocki
2007-03-12 4:38 ` Paul E. McKenney
@ 2007-03-13 5:27 ` Gautham R Shenoy
2007-03-13 5:42 ` Srivatsa Vaddagiri
1 sibling, 1 reply; 46+ messages in thread
From: Gautham R Shenoy @ 2007-03-13 5:27 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Oleg Nesterov, Paul E. McKenney, Anton Blanchard, Andrew Morton,
Pavel Machek, LKML, Aneesh Kumar, Srivatsa Vaddagiri
On Sun, Mar 11, 2007 at 06:49:08PM +0100, Rafael J. Wysocki wrote:
> On Saturday, 3 March 2007 18:32, Oleg Nesterov wrote:
> > On 03/02, Paul E. McKenney wrote:
> > >
> > > On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> > > > On 03/02, Paul E. McKenney wrote:
> > > > >
> > > > > One way to embed try_to_freeze() into kthread_should_stop() might be
> > > > > as follows:
> > > > >
> > > > > int kthread_should_stop(void)
> > > > > {
> > > > > if (kthread_stop_info.k == current)
> > > > > return 1;
> > > > > try_to_freeze();
> > > > > return 0;
> > > > > }
> > > >
> > > > I think this is dangerous. For example, worker_thread() will probably
> > > > need some special actions after return from refrigerator. Also, a kernel
> > > > thread may check kthread_should_stop() in the place where try_to_freeze()
> > > > is not safe.
> > > >
> > > > Perhaps we should introduce a new helper which does this.
> > >
> > > Good point -- the return value from try_to_freeze() is lost if one uses
> > > the above approach. About one third of the calls to try_to_freeze()
> > > in 2.6.20 pay attention to the return value.
> > >
> > > One approach would be to have a kthread_should_stop_nofreeze() for those
> > > cases, and let the default be to try to freeze.
> >
> > I personally think we should do the opposite, add kthread_should_stop_check_freeze()
> > or something. kthread_should_stop() is like signal_pending(), we can use
> > it under spin_lock (and it is probably used this way by some out-of-tree
> > driver). The new helper is obviously "might_sleep()".
>
> Something like this, perhaps:
>
> include/linux/kthread.h | 1 +
> kernel/kthread.c | 16 ++++++++++++++++
> kernel/rcutorture.c | 5 ++---
> 3 files changed, 19 insertions(+), 3 deletions(-)
>
> Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
> ===================================================================
> --- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c 2007-03-08 21:58:48.000000000 +0100
> +++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.000000000 +0100
> @@ -13,6 +13,7 @@
> #include <linux/file.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> +#include <linux/freezer.h>
> #include <asm/semaphore.h>
>
> /*
> @@ -60,6 +61,21 @@ int kthread_should_stop(void)
> }
> EXPORT_SYMBOL(kthread_should_stop);
>
> +/**
> + * kthread_should_stop_check_freeze - check if the thread should return now and
> + * if not, check if there is a freezing request pending for it.
> + */
> +int kthread_should_stop_check_freeze(void)
> +{
> + might_sleep();
> + if (kthread_stop_info.k == current)
> + return 1;
> +
> + try_to_freeze();
> + return 0;
> +}
> +EXPORT_SYMBOL(kthread_should_stop_check_freeze);
I would prefer to have try_to_freeze() followed by the
kthread_stop_info.k check. Something like
if (try_to_freeze())
/*some barrier ensuring all writes are completed */
if (kthread_stop_info.k == current)
return 1;
return 0;
This would be helpful in situations (atleast for cpu-hotplug)
where we want to stop a frozen thread immediately after thawing it.
Something like
CPU_DEAD:
thaw_process(p);
kthread_stop(p);
p = NULL;
Is there a problem with this line of thinking ?
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!"
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
2007-03-13 5:27 ` Gautham R Shenoy
@ 2007-03-13 5:42 ` Srivatsa Vaddagiri
0 siblings, 0 replies; 46+ messages in thread
From: Srivatsa Vaddagiri @ 2007-03-13 5:42 UTC (permalink / raw)
To: Gautham R Shenoy
Cc: Rafael J. Wysocki, Oleg Nesterov, Paul E. McKenney,
Anton Blanchard, Andrew Morton, Pavel Machek, LKML, Aneesh Kumar
On Tue, Mar 13, 2007 at 10:57:16AM +0530, Gautham R Shenoy wrote:
> CPU_DEAD:
> thaw_process(p);
> kthread_stop(p);
> p = NULL;
This neednt guarantee that the thread will see the stop request before
it exits the kthread_should_stop_freeze() function. There will always
be races .. So the only safe way for a thread to know whether it is time
to exit is:
while (!kthread_should_stop_freeze()) {
if (!cpu_online(home_cpu))
goto wait_to_die;
...
}
wait_to_die:
while (!kthread_should_stop()) {
/* sleep */
}
--
Regards,
vatsa
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
2007-03-02 23:33 ` Oleg Nesterov
2007-03-03 0:58 ` Paul E. McKenney
@ 2007-03-03 1:03 ` Rafael J. Wysocki
1 sibling, 0 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2007-03-03 1:03 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Paul E. McKenney, Anton Blanchard, Andrew Morton, Pavel Machek,
LKML, Aneesh Kumar, Srivatsa Vaddagiri, Gautham R Shenoy
On Saturday, 3 March 2007 00:33, Oleg Nesterov wrote:
> On 03/02, Paul E. McKenney wrote:
> >
> > One way to embed try_to_freeze() into kthread_should_stop() might be
> > as follows:
> >
> > int kthread_should_stop(void)
> > {
> > if (kthread_stop_info.k == current)
> > return 1;
> > try_to_freeze();
> > return 0;
> > }
>
> I think this is dangerous. For example, worker_thread() will probably
> need some special actions after return from refrigerator. Also, a kernel
> thread may check kthread_should_stop() in the place where try_to_freeze()
> is not safe.
>
> Perhaps we should introduce a new helper which does this.
Agreed.
Rafael
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
2007-03-01 15:09 ` [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread Rafael J. Wysocki
2007-03-01 19:38 ` Anton Blanchard
@ 2007-03-02 6:57 ` Gautham R Shenoy
2007-03-02 21:46 ` Paul E. McKenney
1 sibling, 1 reply; 46+ messages in thread
From: Gautham R Shenoy @ 2007-03-02 6:57 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Andrew Morton, Pavel Machek, LKML, Oleg Nesterov, Aneesh Kumar,
Paul E. McKenney, Srivatsa Vaddagiri
> From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>
> Remove PF_NOFREEZE from the rcutorture thread, adding a try_to_freeze() call as
> required.
>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
> kernel/rcutorture.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.20-mm2/kernel/rcutorture.c
> ===================================================================
> --- linux-2.6.20-mm2.orig/kernel/rcutorture.c 2007-02-25 12:07:15.000000000 +0100
> +++ linux-2.6.20-mm2/kernel/rcutorture.c 2007-02-25 12:49:23.000000000 +0100
> @@ -46,6 +46,7 @@
> #include <linux/byteorder/swabb.h>
> #include <linux/stat.h>
> #include <linux/srcu.h>
> +#include <linux/freezer.h>
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Paul E. McKenney <paulmck@us.ibm.com> and "
> @@ -585,7 +586,6 @@ rcu_torture_writer(void *arg)
>
> VERBOSE_PRINTK_STRING("rcu_torture_writer task started");
> set_user_nice(current, 19);
> - current->flags |= PF_NOFREEZE;
>
> do {
> schedule_timeout_uninterruptible(1);
> @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
> }
> rcu_torture_current_version++;
> oldbatch = cur_ops->completed();
> + try_to_freeze();
> } while (!kthread_should_stop() && !fullstop);
> VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> while (!kthread_should_stop())
Paul,
Any reasons for not try_to_freeze()'ing the fakewriter and the reader
threads?? (Ok, I admit, I haven't looked into the code for the reason
which might be obvious.)
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!"
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
2007-03-02 6:57 ` Gautham R Shenoy
@ 2007-03-02 21:46 ` Paul E. McKenney
0 siblings, 0 replies; 46+ messages in thread
From: Paul E. McKenney @ 2007-03-02 21:46 UTC (permalink / raw)
To: Gautham R Shenoy
Cc: Rafael J. Wysocki, Andrew Morton, Pavel Machek, LKML,
Oleg Nesterov, Aneesh Kumar, Srivatsa Vaddagiri
On Fri, Mar 02, 2007 at 12:27:30PM +0530, Gautham R Shenoy wrote:
> > From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> >
> > Remove PF_NOFREEZE from the rcutorture thread, adding a try_to_freeze() call as
> > required.
> >
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > Acked-by: Pavel Machek <pavel@ucw.cz>
> > ---
> > kernel/rcutorture.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Index: linux-2.6.20-mm2/kernel/rcutorture.c
> > ===================================================================
> > --- linux-2.6.20-mm2.orig/kernel/rcutorture.c 2007-02-25 12:07:15.000000000 +0100
> > +++ linux-2.6.20-mm2/kernel/rcutorture.c 2007-02-25 12:49:23.000000000 +0100
> > @@ -46,6 +46,7 @@
> > #include <linux/byteorder/swabb.h>
> > #include <linux/stat.h>
> > #include <linux/srcu.h>
> > +#include <linux/freezer.h>
> >
> > MODULE_LICENSE("GPL");
> > MODULE_AUTHOR("Paul E. McKenney <paulmck@us.ibm.com> and "
> > @@ -585,7 +586,6 @@ rcu_torture_writer(void *arg)
> >
> > VERBOSE_PRINTK_STRING("rcu_torture_writer task started");
> > set_user_nice(current, 19);
> > - current->flags |= PF_NOFREEZE;
> >
> > do {
> > schedule_timeout_uninterruptible(1);
> > @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
> > }
> > rcu_torture_current_version++;
> > oldbatch = cur_ops->completed();
> > + try_to_freeze();
> > } while (!kthread_should_stop() && !fullstop);
> > VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> > while (!kthread_should_stop())
>
> Paul,
> Any reasons for not try_to_freeze()'ing the fakewriter and the reader
> threads?? (Ok, I admit, I haven't looked into the code for the reason
> which might be obvious.)
None that I know of -- though I do like Anton's idea of burying a
try_to_freeze() in kthread_should_stop(), which would get rid of all
try_to_freeze() calls in rcutorture, and many other places as well.
Thanx, Paul
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH -mm 4/7] Freezer: Remove PF_NOFREEZE from bluetooth threads
2007-03-01 15:05 ` [PATCH -mm 0/7] Freezer changes (take 2) Rafael J. Wysocki
` (2 preceding siblings ...)
2007-03-01 15:09 ` [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread Rafael J. Wysocki
@ 2007-03-01 15:10 ` Rafael J. Wysocki
2007-03-01 15:12 ` [PATCH -mm 5/7] Freezer: Add try_to_freeze calls to all kernel threads Rafael J. Wysocki
` (2 subsequent siblings)
6 siblings, 0 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2007-03-01 15:10 UTC (permalink / raw)
To: Andrew Morton
Cc: Pavel Machek, LKML, Oleg Nesterov, Aneesh Kumar,
Paul E. McKenney, Srivatsa Vaddagiri, Gautham R Shenoy
From: Rafael J. Wysocki <rjw@sisk.pl>
Remove PF_NOFREEZE from the bluetooth threads, adding try_to_freeze() calls as
required.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
net/bluetooth/bnep/core.c | 4 +++-
net/bluetooth/cmtp/core.c | 4 +++-
net/bluetooth/hidp/core.c | 4 +++-
net/bluetooth/rfcomm/core.c | 4 +++-
4 files changed, 12 insertions(+), 4 deletions(-)
Index: linux-2.6.20-mm2/net/bluetooth/bnep/core.c
===================================================================
--- linux-2.6.20-mm2.orig/net/bluetooth/bnep/core.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/net/bluetooth/bnep/core.c 2007-02-25 12:49:27.000000000 +0100
@@ -39,6 +39,7 @@
#include <linux/errno.h>
#include <linux/smp_lock.h>
#include <linux/net.h>
+#include <linux/freezer.h>
#include <net/sock.h>
#include <linux/socket.h>
@@ -473,11 +474,12 @@ static int bnep_session(void *arg)
daemonize("kbnepd %s", dev->name);
set_user_nice(current, -15);
- current->flags |= PF_NOFREEZE;
init_waitqueue_entry(&wait, current);
add_wait_queue(sk->sk_sleep, &wait);
while (!atomic_read(&s->killed)) {
+ try_to_freeze();
+
set_current_state(TASK_INTERRUPTIBLE);
// RX
Index: linux-2.6.20-mm2/net/bluetooth/cmtp/core.c
===================================================================
--- linux-2.6.20-mm2.orig/net/bluetooth/cmtp/core.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/net/bluetooth/cmtp/core.c 2007-02-25 12:49:27.000000000 +0100
@@ -34,6 +34,7 @@
#include <linux/ioctl.h>
#include <linux/file.h>
#include <linux/init.h>
+#include <linux/freezer.h>
#include <net/sock.h>
#include <linux/isdn/capilli.h>
@@ -287,11 +288,12 @@ static int cmtp_session(void *arg)
daemonize("kcmtpd_ctr_%d", session->num);
set_user_nice(current, -15);
- current->flags |= PF_NOFREEZE;
init_waitqueue_entry(&wait, current);
add_wait_queue(sk->sk_sleep, &wait);
while (!atomic_read(&session->terminate)) {
+ try_to_freeze();
+
set_current_state(TASK_INTERRUPTIBLE);
if (sk->sk_state != BT_CONNECTED)
Index: linux-2.6.20-mm2/net/bluetooth/hidp/core.c
===================================================================
--- linux-2.6.20-mm2.orig/net/bluetooth/hidp/core.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/net/bluetooth/hidp/core.c 2007-02-25 12:49:27.000000000 +0100
@@ -35,6 +35,7 @@
#include <linux/file.h>
#include <linux/init.h>
#include <linux/wait.h>
+#include <linux/freezer.h>
#include <net/sock.h>
#include <linux/input.h>
@@ -547,13 +548,14 @@ static int hidp_session(void *arg)
daemonize("khidpd_%04x%04x", vendor, product);
set_user_nice(current, -15);
- current->flags |= PF_NOFREEZE;
init_waitqueue_entry(&ctrl_wait, current);
init_waitqueue_entry(&intr_wait, current);
add_wait_queue(ctrl_sk->sk_sleep, &ctrl_wait);
add_wait_queue(intr_sk->sk_sleep, &intr_wait);
while (!atomic_read(&session->terminate)) {
+ try_to_freeze();
+
set_current_state(TASK_INTERRUPTIBLE);
if (ctrl_sk->sk_state != BT_CONNECTED || intr_sk->sk_state != BT_CONNECTED)
Index: linux-2.6.20-mm2/net/bluetooth/rfcomm/core.c
===================================================================
--- linux-2.6.20-mm2.orig/net/bluetooth/rfcomm/core.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/net/bluetooth/rfcomm/core.c 2007-02-25 12:49:27.000000000 +0100
@@ -37,6 +37,7 @@
#include <linux/device.h>
#include <linux/net.h>
#include <linux/mutex.h>
+#include <linux/freezer.h>
#include <net/sock.h>
#include <asm/uaccess.h>
@@ -1851,6 +1852,8 @@ static void rfcomm_worker(void)
BT_DBG("");
while (!atomic_read(&terminate)) {
+ try_to_freeze();
+
if (!test_bit(RFCOMM_SCHED_WAKEUP, &rfcomm_event)) {
/* No pending events. Let's sleep.
* Incoming connections and data will wake us up. */
@@ -1937,7 +1940,6 @@ static int rfcomm_run(void *unused)
daemonize("krfcommd");
set_user_nice(current, -10);
- current->flags |= PF_NOFREEZE;
BT_DBG("");
^ permalink raw reply [flat|nested] 46+ messages in thread* [PATCH -mm 5/7] Freezer: Add try_to_freeze calls to all kernel threads
2007-03-01 15:05 ` [PATCH -mm 0/7] Freezer changes (take 2) Rafael J. Wysocki
` (3 preceding siblings ...)
2007-03-01 15:10 ` [PATCH -mm 4/7] Freezer: Remove PF_NOFREEZE from bluetooth threads Rafael J. Wysocki
@ 2007-03-01 15:12 ` Rafael J. Wysocki
2007-03-01 15:15 ` [PATCH -mm 6/7] Freezer: Fix vfork problem Rafael J. Wysocki
2007-03-01 15:17 ` [PATCH -mm 7/7] Freezer: Take kernel_execve into consideration Rafael J. Wysocki
6 siblings, 0 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2007-03-01 15:12 UTC (permalink / raw)
To: Andrew Morton
Cc: Pavel Machek, LKML, Oleg Nesterov, Aneesh Kumar,
Paul E. McKenney, Srivatsa Vaddagiri, Gautham R Shenoy
From: Rafael J. Wysocki <rjw@sisk.pl>
Add try_to_freeze() calls to the remaining kernel threads that do not call
try_to_freeze() already, although they set PF_NOFREEZE.
In the future we are going to replace PF_NOFREEZE with a set of flags that will
be set to indicate in which situations the task should not be frozen (for
example, there can be a task that should be frozen for the CPU hotplugging and
should not be frozen for the system suspend). For this reason every kernel
thread should be able to freeze itself (ie. call try_to_freeze()), so that it
can be frozen whenever necessary.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
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/card/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/softirq.c | 2 ++
kernel/softlockup.c | 2 ++
12 files changed, 30 insertions(+)
Index: linux-2.6.20-mm2/arch/i386/kernel/apm.c
===================================================================
--- linux-2.6.20-mm2.orig/arch/i386/kernel/apm.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/arch/i386/kernel/apm.c 2007-02-25 12:49:30.000000000 +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-mm2/drivers/md/md.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/md/md.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/md/md.c 2007-02-25 12:49:30.000000000 +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-mm2/drivers/mmc/card/queue.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/mmc/card/queue.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/mmc/card/queue.c 2007-02-25 12:49:30.000000000 +0100
@@ -12,6 +12,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>
@@ -71,6 +72,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);
if (!blk_queue_plugged(q))
Index: linux-2.6.20-mm2/drivers/mtd/mtd_blkdevs.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/mtd/mtd_blkdevs.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/mtd/mtd_blkdevs.c 2007-02-25 12:49:30.000000000 +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-mm2/drivers/usb/storage/usb.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/usb/storage/usb.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/usb/storage/usb.c 2007-02-25 12:49:30.000000000 +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(&us->sema))
break;
Index: linux-2.6.20-mm2/drivers/ieee1394/ieee1394_core.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/ieee1394/ieee1394_core.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/ieee1394/ieee1394_core.c 2007-02-25 12:49:30.000000000 +0100
@@ -35,6 +35,7 @@
#include <linux/kthread.h>
#include <linux/preempt.h>
#include <linux/time.h>
+#include <linux/freezer.h>
#include <asm/system.h>
#include <asm/byteorder.h>
@@ -1081,6 +1082,8 @@ static int hpsbpkt_thread(void *__hi)
complete_routine(complete_data);
}
+ try_to_freeze();
+
set_current_state(TASK_INTERRUPTIBLE);
if (!skb_peek(&hpsbpkt_queue))
schedule();
Index: linux-2.6.20-mm2/drivers/char/apm-emulation.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/char/apm-emulation.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/char/apm-emulation.c 2007-02-25 12:49:30.000000000 +0100
@@ -27,6 +27,7 @@
#include <linux/completion.h>
#include <linux/kthread.h>
#include <linux/delay.h>
+#include <linux/freezer.h>
#include <asm/system.h>
@@ -539,6 +540,8 @@ static int kapmd(void *arg)
apm_event_t event;
int ret;
+ try_to_freeze();
+
wait_event_interruptible(kapmd_wait,
!queue_empty(&kapmd_queue) || kthread_should_stop());
Index: linux-2.6.20-mm2/drivers/block/loop.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/block/loop.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/block/loop.c 2007-02-25 12:49:30.000000000 +0100
@@ -74,6 +74,7 @@
#include <linux/highmem.h>
#include <linux/gfp.h>
#include <linux/kthread.h>
+#include <linux/freezer.h>
#include <asm/uaccess.h>
@@ -580,6 +581,7 @@ static int loop_thread(void *data)
set_user_nice(current, -20);
while (!kthread_should_stop() || lo->lo_bio) {
+ try_to_freeze();
wait_event_interruptible(lo->lo_event,
lo->lo_bio || kthread_should_stop());
Index: linux-2.6.20-mm2/drivers/scsi/libsas/sas_scsi_host.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/scsi/libsas/sas_scsi_host.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/scsi/libsas/sas_scsi_host.c 2007-02-25 12:49:30.000000000 +0100
@@ -39,6 +39,7 @@
#include <linux/err.h>
#include <linux/blkdev.h>
#include <linux/scatterlist.h>
+#include <linux/freezer.h>
/* ---------- SCSI Host glue ---------- */
@@ -875,6 +876,8 @@ static int sas_queue_thread(void *_sas_h
complete(&queue_th_comp);
while (1) {
+ try_to_freeze();
+
down_interruptible(&core->queue_thread_sema);
sas_queue(sas_ha);
if (core->queue_thread_kill)
Index: linux-2.6.20-mm2/drivers/scsi/scsi_error.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/scsi/scsi_error.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/scsi/scsi_error.c 2007-02-25 12:49:30.000000000 +0100
@@ -24,6 +24,7 @@
#include <linux/interrupt.h>
#include <linux/blkdev.h>
#include <linux/delay.h>
+#include <linux/freezer.h>
#include <scsi/scsi.h>
#include <scsi/scsi_cmnd.h>
@@ -1536,6 +1537,8 @@ int scsi_error_handler(void *data)
*/
set_current_state(TASK_INTERRUPTIBLE);
while (!kthread_should_stop()) {
+ try_to_freeze();
+
if ((shost->host_failed == 0 && shost->host_eh_scheduled == 0) ||
shost->host_failed != shost->host_busy) {
SCSI_LOG_ERROR_RECOVERY(1,
Index: linux-2.6.20-mm2/kernel/softlockup.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/softlockup.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/kernel/softlockup.c 2007-02-25 12:49:30.000000000 +0100
@@ -13,6 +13,7 @@
#include <linux/kthread.h>
#include <linux/notifier.h>
#include <linux/module.h>
+#include <linux/freezer.h>
static DEFINE_SPINLOCK(print_lock);
@@ -93,6 +94,7 @@ static int watchdog(void * __bind_cpu)
* debug-printout triggers in softlockup_tick().
*/
while (!kthread_should_stop()) {
+ try_to_freeze();
set_current_state(TASK_INTERRUPTIBLE);
touch_softlockup_watchdog();
schedule();
Index: linux-2.6.20-mm2/kernel/softirq.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/softirq.c 2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/kernel/softirq.c 2007-02-25 12:49:30.000000000 +0100
@@ -18,6 +18,7 @@
#include <linux/rcupdate.h>
#include <linux/smp.h>
#include <linux/tick.h>
+#include <linux/freezer.h>
#include <asm/irq.h>
/*
@@ -494,6 +495,7 @@ static int ksoftirqd(void * __bind_cpu)
set_current_state(TASK_INTERRUPTIBLE);
while (!kthread_should_stop()) {
+ try_to_freeze();
preempt_disable();
if (!local_softirq_pending()) {
preempt_enable_no_resched();
^ permalink raw reply [flat|nested] 46+ messages in thread* [PATCH -mm 6/7] Freezer: Fix vfork problem
2007-03-01 15:05 ` [PATCH -mm 0/7] Freezer changes (take 2) Rafael J. Wysocki
` (4 preceding siblings ...)
2007-03-01 15:12 ` [PATCH -mm 5/7] Freezer: Add try_to_freeze calls to all kernel threads Rafael J. Wysocki
@ 2007-03-01 15:15 ` Rafael J. Wysocki
2007-03-01 15:17 ` [PATCH -mm 7/7] Freezer: Take kernel_execve into consideration Rafael J. Wysocki
6 siblings, 0 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2007-03-01 15:15 UTC (permalink / raw)
To: Andrew Morton
Cc: Pavel Machek, LKML, Oleg Nesterov, Aneesh Kumar,
Paul E. McKenney, Srivatsa Vaddagiri, Gautham R Shenoy
From: Rafael J. Wysocki <rjw@sisk.pl>
Currently try_to_freeze_tasks() has to wait until all of the vforked processes
exit and for this reason every user can make it fail. To fix this problem
we can introduce the additional process flag PF_FREEZER_SKIP to be used by
tasks that do not want to be counted as freezable by the freezer and want to
have TIF_FREEZE set nevertheless. Then, this flag can be set by tasks using
sys_vfork() before they call wait_for_completion(&vfork) and cleared after they
have woken up. After clearing it, the tasks should call try_to_freeze() as
soon as possible.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
include/linux/freezer.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
include/linux/sched.h | 1 +
kernel/fork.c | 3 +++
kernel/power/process.c | 27 ++++++++-------------------
4 files changed, 58 insertions(+), 21 deletions(-)
Index: linux-2.6.20-mm2/include/linux/sched.h
===================================================================
--- linux-2.6.20-mm2.orig/include/linux/sched.h
+++ linux-2.6.20-mm2/include/linux/sched.h
@@ -1189,6 +1189,7 @@ static inline void put_task_struct(struc
#define PF_SPREAD_SLAB 0x02000000 /* Spread some slab caches over cpuset */
#define PF_MEMPOLICY 0x10000000 /* Non-default NUMA mempolicy */
#define PF_MUTEX_TESTER 0x20000000 /* Thread belongs to the rt mutex tester */
+#define PF_FREEZER_SKIP 0x40000000 /* Freezer should not count it as freezeable */
/*
* Only the _current_ task can read/write to tsk->flags, but other
Index: linux-2.6.20-mm2/include/linux/freezer.h
===================================================================
--- linux-2.6.20-mm2.orig/include/linux/freezer.h
+++ linux-2.6.20-mm2/include/linux/freezer.h
@@ -75,7 +75,49 @@ static inline int try_to_freeze(void)
return 0;
}
-extern void thaw_some_processes(int all);
+/*
+ * The PF_FREEZER_SKIP flag should be set by a vfork parent right before it
+ * calls wait_for_completion(&vfork) and reset right after it returns from this
+ * function. Next, the parent should call try_to_freeze() to freeze itself
+ * appropriately in case the child has exited before the freezing of tasks is
+ * complete. However, we don't want kernel threads to be frozen in unexpected
+ * places, so we allow them to block freeze_processes() instead or to set
+ * PF_NOFREEZE if needed and PF_FREEZER_SKIP is only set for userland vfork
+ * parents. Fortunately, in the ____call_usermodehelper() case the parent won't
+ * really block freeze_processes(), since ____call_usermodehelper() (the child)
+ * does a little before exec/exit and it can't be frozen before waking up the
+ * parent.
+ */
+
+/*
+ * If the current task is a user space one, tell the freezer not to count it as
+ * freezable.
+ */
+static inline void freezer_do_not_count(void)
+{
+ if (current->mm)
+ current->flags |= PF_FREEZER_SKIP;
+}
+
+/*
+ * If the current task is a user space one, tell the freezer to count it as
+ * freezable again and try to freeze it.
+ */
+static inline void freezer_count(void)
+{
+ if (current->mm) {
+ current->flags &= ~PF_FREEZER_SKIP;
+ try_to_freeze();
+ }
+}
+
+/*
+ * Check if the task should be counted as freezeable by the freezer
+ */
+static inline int freezer_should_skip(struct task_struct *p)
+{
+ return !!(p->flags & PF_FREEZER_SKIP);
+}
#else
static inline int frozen(struct task_struct *p) { return 0; }
@@ -90,5 +132,7 @@ static inline void thaw_processes(void)
static inline int try_to_freeze(void) { return 0; }
-
+static inline void freezer_do_not_count(void) {}
+static inline void freezer_count(void) {}
+static inline int freezer_should_skip(struct task_struct *p) { return 0; }
#endif
Index: linux-2.6.20-mm2/kernel/fork.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/fork.c
+++ linux-2.6.20-mm2/kernel/fork.c
@@ -50,6 +50,7 @@
#include <linux/taskstats_kern.h>
#include <linux/random.h>
#include <linux/ptrace.h>
+#include <linux/freezer.h>
#include <asm/pgtable.h>
#include <asm/pgalloc.h>
@@ -1393,7 +1394,9 @@ long do_fork(unsigned long clone_flags,
tracehook_report_clone_complete(clone_flags, nr, p);
if (clone_flags & CLONE_VFORK) {
+ freezer_do_not_count();
wait_for_completion(&vfork);
+ freezer_count();
tracehook_report_vfork_done(p, nr);
}
} else {
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
@@ -127,22 +127,12 @@ 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);
- }
- todo++;
+ freeze_process(p);
+ if (!freezer_should_skip(p))
+ todo++;
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
yield(); /* Yield is okay here */
@@ -168,7 +158,8 @@ static unsigned int try_to_freeze_tasks(
continue;
task_lock(p);
- if (freezeable(p) && !frozen(p))
+ if (freezeable(p) && !frozen(p) &&
+ !freezer_should_skip(p))
printk(KERN_ERR " %s\n", p->comm);
cancel_freezing(p);
@@ -217,9 +208,7 @@ static void thaw_tasks(int thaw_user_spa
if (is_user_space(p) == !thaw_user_space)
continue;
- if (!thaw_process(p))
- printk(KERN_WARNING " Strange, %s not stopped\n",
- p->comm );
+ thaw_process(p);
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
}
^ permalink raw reply [flat|nested] 46+ messages in thread* [PATCH -mm 7/7] Freezer: Take kernel_execve into consideration
2007-03-01 15:05 ` [PATCH -mm 0/7] Freezer changes (take 2) Rafael J. Wysocki
` (5 preceding siblings ...)
2007-03-01 15:15 ` [PATCH -mm 6/7] Freezer: Fix vfork problem Rafael J. Wysocki
@ 2007-03-01 15:17 ` Rafael J. Wysocki
6 siblings, 0 replies; 46+ messages in thread
From: Rafael J. Wysocki @ 2007-03-01 15:17 UTC (permalink / raw)
To: Andrew Morton
Cc: Pavel Machek, LKML, Oleg Nesterov, Aneesh Kumar,
Paul E. McKenney, Srivatsa Vaddagiri, Gautham R Shenoy
From: Rafael J. Wysocki <rjw@sisk.pl>
Kernel threads can become userland processes by calling kernel_execve(). In
particular, this may happen right after try_to_freeze_tasks(FREEZER_USER_SPACE)
has returned, so try_to_freeze_tasks() needs to take userspace processes
into consideration even if it is called with FREEZER_KERNEL_THREADS.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
kernel/power/process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
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
@@ -127,7 +127,7 @@ static unsigned int try_to_freeze_tasks(
cancel_freezing(p);
continue;
}
- if (is_user_space(p) == !freeze_user_space)
+ if (freeze_user_space && !is_user_space(p))
continue;
freeze_process(p);
@@ -154,7 +154,7 @@ static unsigned int try_to_freeze_tasks(
TIMEOUT / HZ, todo);
read_lock(&tasklist_lock);
do_each_thread(g, p) {
- if (is_user_space(p) == !freeze_user_space)
+ if (freeze_user_space && !is_user_space(p))
continue;
task_lock(p);
^ permalink raw reply [flat|nested] 46+ messages in thread