Re: 2.6.21-rc4-mm1

Re: 2.6.21-rc4-mm1

[Jiri Slaby]

Andrew Morton napsal(a):
>   http://userweb.kernel.org/~akpm/2.6.21-rc4-mm1/

I'm getting this while trying to swsusp:
Stopping tasks ...
Stopping kernel threads timed out after 20 seconds (1 tasks refusing to freeze):
 swapper
 Restarting tasks ... done.

What to test? Enable PM_DEBUG?

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: 2.6.21-rc4-mm1

[Andrew Morton]

On Tue, 20 Mar 2007 15:31:02 +0100 Jiri Slaby <jirislaby@gmail.com> wrote:

> Andrew Morton napsal(a):
> >   http://userweb.kernel.org/~akpm/2.6.21-rc4-mm1/
> 
> I'm getting this while trying to swsusp:
> Stopping tasks ...
> Stopping kernel threads timed out after 20 seconds (1 tasks refusing to freeze):
>  swapper
>  Restarting tasks ... done.
> 
> What to test? Enable PM_DEBUG?

hm, OK.  Rafael has been working on fixing the process freezer and it'll
take some time to get it to where we want it to be, I expect.

Rafael, I think that we could afford to add heaps of debug in there at this
stage to help us track down problems like this.

Also, it might be useful to add a temporary /proc/freeze-unfreeze thing
which will simply do a freeze/unfreeze cycle.  Then we can apply various
workloads to the machine while madly stressing the freezer code.  

Re: 2.6.21-rc4-mm1

[Pavel Machek]

Hi!

> > Andrew Morton napsal(a):
> > >   http://userweb.kernel.org/~akpm/2.6.21-rc4-mm1/
> > 
> > I'm getting this while trying to swsusp:
> > Stopping tasks ...
> > Stopping kernel threads timed out after 20 seconds (1 tasks refusing to freeze):
> >  swapper
> >  Restarting tasks ... done.
> > 
> > What to test? Enable PM_DEBUG?

Is it repeatable? Freezer normally works better than that.

> hm, OK.  Rafael has been working on fixing the process freezer and it'll
> take some time to get it to where we want it to be, I expect.

> Rafael, I think that we could afford to add heaps of debug in there at this
> stage to help us track down problems like this.
> 
> Also, it might be useful to add a temporary /proc/freeze-unfreeze thing
> which will simply do a freeze/unfreeze cycle.  Then we can apply various
> workloads to the machine while madly stressing the freezer code.  

echo testproc > /sys/power/disk; echo disk > /sys/power/state ... is
pretty much what you want.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: 2.6.21-rc4-mm1

[Jiri Slaby]

Pavel Machek napsal(a):
> Hi!
> 
>>> Andrew Morton napsal(a):
>>>>   http://userweb.kernel.org/~akpm/2.6.21-rc4-mm1/
>>> I'm getting this while trying to swsusp:
>>> Stopping tasks ...
>>> Stopping kernel threads timed out after 20 seconds (1 tasks refusing to freeze):
>>>  swapper
>>>  Restarting tasks ... done.
>>>
>>> What to test? Enable PM_DEBUG?
> 
> Is it repeatable? Freezer normally works better than that.

Yes, at least it happened 3 times consecutively, when I tried to asleep the
machine, then I turned it off.

>> Also, it might be useful to add a temporary /proc/freeze-unfreeze thing
>> which will simply do a freeze/unfreeze cycle.  Then we can apply various
>> workloads to the machine while madly stressing the freezer code.  
> 
> echo testproc > /sys/power/disk; echo disk > /sys/power/state ... is
> pretty much what you want.

Ok, I'll try this.

thanks,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: 2.6.21-rc4-mm1

[Pavel Machek]

Hi!

> >> Also, it might be useful to add a temporary /proc/freeze-unfreeze thing
> >> which will simply do a freeze/unfreeze cycle.  Then we can apply various
> >> workloads to the machine while madly stressing the freezer code.  
> > 
> > echo testproc > /sys/power/disk; echo disk > /sys/power/state ... is
> > pretty much what you want.
> 
> Ok, I'll try this.

It will not help you -- probably -- it is equivalent to just running
s2ram. But it should make "successful" testing easier, because you no
longer need machine with working suspend to test refrigerator.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: 2.6.21-rc4-mm1

[Rafael J. Wysocki]

On Tuesday, 20 March 2007 20:40, Jiri Slaby wrote:
> Pavel Machek napsal(a):
> > Hi!
> > 
> >>> Andrew Morton napsal(a):
> >>>>   http://userweb.kernel.org/~akpm/2.6.21-rc4-mm1/
> >>> I'm getting this while trying to swsusp:
> >>> Stopping tasks ...
> >>> Stopping kernel threads timed out after 20 seconds (1 tasks refusing to freeze):
> >>>  swapper
> >>>  Restarting tasks ... done.
> >>>
> >>> What to test? Enable PM_DEBUG?
> > 
> > Is it repeatable? Freezer normally works better than that.
> 
> Yes, at least it happened 3 times consecutively, when I tried to asleep the
> machine, then I turned it off.
> 
> >> Also, it might be useful to add a temporary /proc/freeze-unfreeze thing
> >> which will simply do a freeze/unfreeze cycle.  Then we can apply various
> >> workloads to the machine while madly stressing the freezer code.  
> > 
> > echo testproc > /sys/power/disk; echo disk > /sys/power/state ... is
> > pretty much what you want.
> 
> Ok, I'll try this.

Can I see .config please?

Rafael

Re: 2.6.21-rc4-mm1

[Jiri Slaby]

Pavel Machek napsal(a):
> Hi!
> 
>>>> Also, it might be useful to add a temporary /proc/freeze-unfreeze thing
>>>> which will simply do a freeze/unfreeze cycle.  Then we can apply various
>>>> workloads to the machine while madly stressing the freezer code.  
>>> echo testproc > /sys/power/disk; echo disk > /sys/power/state ... is
>>> pretty much what you want.
>> Ok, I'll try this.
> 
> It will not help you -- probably -- it is equivalent to just running
> s2ram. But it should make "successful" testing easier, because you no
> longer need machine with working suspend to test refrigerator.

Aha, I didn't read it carefully. Suspend is working, but not in this kernel.
I haven't tried s2ram in this version. Should I (I'm away from it) -- would
it show something?

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: 2.6.21-rc4-mm1

[Pavel Machek]

Hi!

> >>>> Also, it might be useful to add a temporary /proc/freeze-unfreeze thing
> >>>> which will simply do a freeze/unfreeze cycle.  Then we can apply various
> >>>> workloads to the machine while madly stressing the freezer code.  
> >>> echo testproc > /sys/power/disk; echo disk > /sys/power/state ... is
> >>> pretty much what you want.
> >> Ok, I'll try this.
> > 
> > It will not help you -- probably -- it is equivalent to just running
> > s2ram. But it should make "successful" testing easier, because you no
> > longer need machine with working suspend to test refrigerator.
> 
> Aha, I didn't read it carefully. Suspend is working, but not in this kernel.
> I haven't tried s2ram in this version. Should I (I'm away from it) -- would
> it show something?

No, probably not. git bisect would help, but I guess it is easier to
let Rafael sort it out.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: 2.6.21-rc4-mm1

[Jiri Slaby]

Rafael J. Wysocki napsal(a):
> Actually, the problem is 100% reproducible on my system too and I doubt it's
> caused by the recent freezer patches.

I don't know what exactly do you mean by recent, but 2.6.21-rc3-mm2 works
for me.

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: 2.6.21-rc4-mm1

[Rafael J. Wysocki]

On Tuesday, 20 March 2007 21:21, Pavel Machek wrote:
> Hi!
> 
> > >>>> Also, it might be useful to add a temporary /proc/freeze-unfreeze thing
> > >>>> which will simply do a freeze/unfreeze cycle.  Then we can apply various
> > >>>> workloads to the machine while madly stressing the freezer code.  
> > >>> echo testproc > /sys/power/disk; echo disk > /sys/power/state ... is
> > >>> pretty much what you want.
> > >> Ok, I'll try this.
> > > 
> > > It will not help you -- probably -- it is equivalent to just running
> > > s2ram. But it should make "successful" testing easier, because you no
> > > longer need machine with working suspend to test refrigerator.
> > 
> > Aha, I didn't read it carefully. Suspend is working, but not in this kernel.
> > I haven't tried s2ram in this version. Should I (I'm away from it) -- would
> > it show something?
> 
> No, probably not. git bisect would help, but I guess it is easier to
> let Rafael sort it out.

Actually, the problem is 100% reproducible on my system too and I doubt it's
caused by the recent freezer patches.

Investigating.

Rafael

Re: 2.6.21-rc4-mm1

[Rafael J. Wysocki]

On Tuesday, 20 March 2007 21:58, Jiri Slaby wrote:
> Rafael J. Wysocki napsal(a):
> > Actually, the problem is 100% reproducible on my system too and I doubt it's
> > caused by the recent freezer patches.
> 
> I don't know what exactly do you mean by recent, but 2.6.21-rc3-mm2 works
> for me.

Thanks for the confirmation.

The patches I was talking about had already been in 2.6.21-rc3-mm2, so the
reason of this failure must be different.

Greetings,
Rafael

Re: 2.6.21-rc4-mm1: freezing of processes broken

[Rafael J. Wysocki]

On Tuesday, 20 March 2007 22:06, Rafael J. Wysocki wrote:
> On Tuesday, 20 March 2007 21:58, Jiri Slaby wrote:
> > Rafael J. Wysocki napsal(a):
> > > Actually, the problem is 100% reproducible on my system too and I doubt it's
> > > caused by the recent freezer patches.
> > 
> > I don't know what exactly do you mean by recent, but 2.6.21-rc3-mm2 works
> > for me.
> 
> Thanks for the confirmation.
> 
> The patches I was talking about had already been in 2.6.21-rc3-mm2, so the
> reason of this failure must be different.

Bisection shows that the freezing of processes has been broken by one of the
patches:

remove-the-likelypid-check-in-copy_process.patch
use-task_pgrp-task_session-in-copy_process.patch

Greetings,
Rafael

Re: 2.6.21-rc4-mm1: freezing of processes broken

[Eric W. Biederman]

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

> On Tuesday, 20 March 2007 22:06, Rafael J. Wysocki wrote:
>> On Tuesday, 20 March 2007 21:58, Jiri Slaby wrote:
>> > Rafael J. Wysocki napsal(a):
>> > > Actually, the problem is 100% reproducible on my system too and I doubt
> it's
>> > > caused by the recent freezer patches.
>> > 
>> > I don't know what exactly do you mean by recent, but 2.6.21-rc3-mm2 works
>> > for me.
>> 
>> Thanks for the confirmation.
>> 
>> The patches I was talking about had already been in 2.6.21-rc3-mm2, so the
>> reason of this failure must be different.
>
> Bisection shows that the freezing of processes has been broken by one of the
> patches:
>
> remove-the-likelypid-check-in-copy_process.patch

Grr.  Oleg's review of remove-the-likelypid-check-in-copy-process
showed it to be questionable (and it was just an optimization)
so we can get rid of that one easily. 

Although all it did that was really questionable was add
the idle process to the global process list and bump a process
count when we forked the idle process.  Not dramatically dangerous
things.

> use-task_pgrp-task_session-in-copy_process.patch

As I recall that patch was pretty trivial, and shouldn't have
anything to do with the freezer.   The process freezer doesn't care
about pids does it?

Eric

Re: 2.6.21-rc4-mm1: freezing of processes broken

[sukadev]

Eric W. Biederman [ebiederm@xmission.com] wrote:
| "Rafael J. Wysocki" <rjw@sisk.pl> writes:
| 
| > On Tuesday, 20 March 2007 22:06, Rafael J. Wysocki wrote:
| >> On Tuesday, 20 March 2007 21:58, Jiri Slaby wrote:
| >> > Rafael J. Wysocki napsal(a):
| >> > > Actually, the problem is 100% reproducible on my system too and I doubt
| > it's
| >> > > caused by the recent freezer patches.
| >> > 
| >> > I don't know what exactly do you mean by recent, but 2.6.21-rc3-mm2 works
| >> > for me.
| >> 
| >> Thanks for the confirmation.
| >> 
| >> The patches I was talking about had already been in 2.6.21-rc3-mm2, so the
| >> reason of this failure must be different.
| >
| > Bisection shows that the freezing of processes has been broken by one of the
| > patches:
| >
| > remove-the-likelypid-check-in-copy_process.patch
| 
| Grr.  Oleg's review of remove-the-likelypid-check-in-copy-process
| showed it to be questionable (and it was just an optimization)
| so we can get rid of that one easily. 
| 
| Although all it did that was really questionable was add
| the idle process to the global process list and bump a process
| count when we forked the idle process.  Not dramatically dangerous
| things.
| 
| > use-task_pgrp-task_session-in-copy_process.patch
| 
| As I recall that patch was pretty trivial, and shouldn't have
| anything to do with the freezer.   The process freezer doesn't care
| about pids does it?

Yes. I think this one is trivial too. Here is the effective change in
copy_process():

-               attach_pid(p, PIDTYPE_PGID, find_pid(pgid));
-               attach_pid(p, PIDTYPE_SID, find_pid(sid));
+               attach_pid(p, PIDTYPE_PGID, task_pgrp(current));
+               attach_pid(p, PIDTYPE_SID, task_session(current));


| 
| Eric

Re: 2.6.21-rc4-mm1: freezing of processes broken

[Nigel Cunningham]

Hi.

On Tue, 2007-03-20 at 19:23 -0600, Eric W. Biederman wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> 
> > On Tuesday, 20 March 2007 22:06, Rafael J. Wysocki wrote:
> >> On Tuesday, 20 March 2007 21:58, Jiri Slaby wrote:
> >> > Rafael J. Wysocki napsal(a):
> >> > > Actually, the problem is 100% reproducible on my system too and I doubt
> > it's
> >> > > caused by the recent freezer patches.
> >> > 
> >> > I don't know what exactly do you mean by recent, but 2.6.21-rc3-mm2 works
> >> > for me.
> >> 
> >> Thanks for the confirmation.
> >> 
> >> The patches I was talking about had already been in 2.6.21-rc3-mm2, so the
> >> reason of this failure must be different.
> >
> > Bisection shows that the freezing of processes has been broken by one of the
> > patches:
> >
> > remove-the-likelypid-check-in-copy_process.patch
> 
> Grr.  Oleg's review of remove-the-likelypid-check-in-copy-process
> showed it to be questionable (and it was just an optimization)
> so we can get rid of that one easily. 
> 
> Although all it did that was really questionable was add
> the idle process to the global process list and bump a process
> count when we forked the idle process.  Not dramatically dangerous
> things.
> 
> > use-task_pgrp-task_session-in-copy_process.patch
> 
> As I recall that patch was pretty trivial, and shouldn't have
> anything to do with the freezer.   The process freezer doesn't care
> about pids does it?

Could the freezer code be trying to freeze the idle thread as a result?

Regards,

Nigel

Re: 2.6.21-rc4-mm1: freezing of processes broken

[Rafael J. Wysocki]

On Wednesday, 21 March 2007 02:23, Eric W. Biederman wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> 
> > On Tuesday, 20 March 2007 22:06, Rafael J. Wysocki wrote:
> >> On Tuesday, 20 March 2007 21:58, Jiri Slaby wrote:
> >> > Rafael J. Wysocki napsal(a):
> >> > > Actually, the problem is 100% reproducible on my system too and I doubt
> > it's
> >> > > caused by the recent freezer patches.
> >> > 
> >> > I don't know what exactly do you mean by recent, but 2.6.21-rc3-mm2 works
> >> > for me.
> >> 
> >> Thanks for the confirmation.
> >> 
> >> The patches I was talking about had already been in 2.6.21-rc3-mm2, so the
> >> reason of this failure must be different.
> >
> > Bisection shows that the freezing of processes has been broken by one of the
> > patches:
> >
> > remove-the-likelypid-check-in-copy_process.patch
> 
> Grr.  Oleg's review of remove-the-likelypid-check-in-copy-process
> showed it to be questionable (and it was just an optimization)
> so we can get rid of that one easily. 
> 
> Although all it did that was really questionable was add
> the idle process to the global process list and bump a process
> count when we forked the idle process.  Not dramatically dangerous
> things.

I think this patch is the culprit, though, as it seems that the freezer tries
to freeze the idle process and fails.  Yesterday I didn't have the time to
figure out the exact reason, I'll try to look at it later today.

> > use-task_pgrp-task_session-in-copy_process.patch
> 
> As I recall that patch was pretty trivial, and shouldn't have
> anything to do with the freezer.   The process freezer doesn't care
> about pids does it?

No, it doesn't, at least not directly.  I'll try to run the kernel with this
change and without remove-the-likelypid-check-in-copy_process.patch .

Greetings,
Rafael

Re: 2.6.21-rc4-mm1: freezing of processes broken

[Oleg Nesterov]

On 03/21, Nigel Cunningham wrote:
> 
> On Tue, 2007-03-20 at 19:23 -0600, Eric W. Biederman wrote:
> > >> > Rafael J. Wysocki napsal(a):
> > >
> > > Bisection shows that the freezing of processes has been broken by one of the
> > > patches:
> > >
> > > remove-the-likelypid-check-in-copy_process.patch
> > 
> > Grr.  Oleg's review of remove-the-likelypid-check-in-copy-process
> > showed it to be questionable (and it was just an optimization)
> > so we can get rid of that one easily. 
> > 
> > Although all it did that was really questionable was add
> > the idle process to the global process list and bump a process
> > count when we forked the idle process.  Not dramatically dangerous
> > things.
> 
> Could the freezer code be trying to freeze the idle thread as a result?

Yes. remove-the-likelypid-check-in-copy_process.patch make idle threads
visible to for_each_process/do_each_thread. Note also that idle threads
have ->mm != NULL. freezer, oom_kill, move_task_off_dead_cpu, etc, should
not see idle threads, but they do now.

Oleg.

Re: 2.6.21-rc4-mm1: freezing of processes broken

[Rafael J. Wysocki]

On Wednesday, 21 March 2007 15:23, Oleg Nesterov wrote:
> On 03/21, Nigel Cunningham wrote:
> > 
> > On Tue, 2007-03-20 at 19:23 -0600, Eric W. Biederman wrote:
> > > >> > Rafael J. Wysocki napsal(a):
> > > >
> > > > Bisection shows that the freezing of processes has been broken by one of the
> > > > patches:
> > > >
> > > > remove-the-likelypid-check-in-copy_process.patch
> > > 
> > > Grr.  Oleg's review of remove-the-likelypid-check-in-copy-process
> > > showed it to be questionable (and it was just an optimization)
> > > so we can get rid of that one easily. 
> > > 
> > > Although all it did that was really questionable was add
> > > the idle process to the global process list and bump a process
> > > count when we forked the idle process.  Not dramatically dangerous
> > > things.
> > 
> > Could the freezer code be trying to freeze the idle thread as a result?
> 
> Yes. remove-the-likelypid-check-in-copy_process.patch make idle threads
> visible to for_each_process/do_each_thread. Note also that idle threads
> have ->mm != NULL. freezer, oom_kill, move_task_off_dead_cpu, etc, should
> not see idle threads, but they do now.

Well, I think this is a good enough reason for asking Andrew to drop this
patch.

Greetings,
Rafael

Re: 2.6.21-rc4-mm1: freezing of processes broken

[Andrew Morton]

On Wed, 21 Mar 2007 15:45:16 +0100 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> On Wednesday, 21 March 2007 15:23, Oleg Nesterov wrote:
> > On 03/21, Nigel Cunningham wrote:
> > > 
> > > On Tue, 2007-03-20 at 19:23 -0600, Eric W. Biederman wrote:
> > > > >> > Rafael J. Wysocki napsal(a):
> > > > >
> > > > > Bisection shows that the freezing of processes has been broken by one of the
> > > > > patches:
> > > > >
> > > > > remove-the-likelypid-check-in-copy_process.patch
> > > > 
> > > > Grr.  Oleg's review of remove-the-likelypid-check-in-copy-process
> > > > showed it to be questionable (and it was just an optimization)
> > > > so we can get rid of that one easily. 
> > > > 
> > > > Although all it did that was really questionable was add
> > > > the idle process to the global process list and bump a process
> > > > count when we forked the idle process.  Not dramatically dangerous
> > > > things.
> > > 
> > > Could the freezer code be trying to freeze the idle thread as a result?
> > 
> > Yes. remove-the-likelypid-check-in-copy_process.patch make idle threads
> > visible to for_each_process/do_each_thread. Note also that idle threads
> > have ->mm != NULL. freezer, oom_kill, move_task_off_dead_cpu, etc, should
> > not see idle threads, but they do now.
> 
> Well, I think this is a good enough reason for asking Andrew to drop this
> patch.

Or we change the freezer to skip pid==0 tasks.

Re: 2.6.21-rc4-mm1: freezing of processes broken

[Oleg Nesterov]

On 03/21, Andrew Morton wrote:
>
> On Wed, 21 Mar 2007 15:45:16 +0100 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > On Wednesday, 21 March 2007 15:23, Oleg Nesterov wrote:
> > > > 
> > > > Could the freezer code be trying to freeze the idle thread as a result?
> > > 
> > > Yes. remove-the-likelypid-check-in-copy_process.patch make idle threads
> > > visible to for_each_process/do_each_thread. Note also that idle threads
> > > have ->mm != NULL. freezer, oom_kill, move_task_off_dead_cpu, etc, should
> > > not see idle threads, but they do now.
> > 
> > Well, I think this is a good enough reason for asking Andrew to drop this
> > patch.
> 
> Or we change the freezer to skip pid==0 tasks.

I strongly disagree. In that case we should audit all users of for_each_process.
Imho, this change is too dangerous. Actually, I personally think it is very good
that idle threads are special and not visible, imho we should not change this.

Btw. Rafael, Andrew, what about

	[PATCH] Make XFS workqueues nonfreezable
	http://marc.info/?l=linux-kernel&m=117270675922229

, don't you think this should go to 2.6.21 ?

Oleg.

Re: 2.6.21-rc4-mm1: freezing of processes broken

[Andrew Morton]

On Wed, 21 Mar 2007 19:55:34 +0300 Oleg Nesterov <oleg@tv-sign.ru> wrote:

> On 03/21, Andrew Morton wrote:
> >
> > On Wed, 21 Mar 2007 15:45:16 +0100 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > 
> > > On Wednesday, 21 March 2007 15:23, Oleg Nesterov wrote:
> > > > > 
> > > > > Could the freezer code be trying to freeze the idle thread as a result?
> > > > 
> > > > Yes. remove-the-likelypid-check-in-copy_process.patch make idle threads
> > > > visible to for_each_process/do_each_thread. Note also that idle threads
> > > > have ->mm != NULL. freezer, oom_kill, move_task_off_dead_cpu, etc, should
> > > > not see idle threads, but they do now.
> > > 
> > > Well, I think this is a good enough reason for asking Andrew to drop this
> > > patch.
> > 
> > Or we change the freezer to skip pid==0 tasks.
> 
> I strongly disagree. In that case we should audit all users of for_each_process.
> Imho, this change is too dangerous. Actually, I personally think it is very good
> that idle threads are special and not visible, imho we should not change this.

OK, I'll tentatively drop remove-the-likelypid-check-in-copy_process.patch

> Btw. Rafael, Andrew, what about
> 
> 	[PATCH] Make XFS workqueues nonfreezable
> 	http://marc.info/?l=linux-kernel&m=117270675922229
> 
> , don't you think this should go to 2.6.21 ?
> 

I've sent it to the XFS guys a couple of times, but their black hole is one
of the deeper ones.

David?

Re: 2.6.21-rc4-mm1: freezing of processes broken

[Oleg Nesterov]

On 03/21, Oleg Nesterov wrote:
>
> On 03/21, Andrew Morton wrote:
> >
> > On Wed, 21 Mar 2007 15:45:16 +0100 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > 
> > > On Wednesday, 21 March 2007 15:23, Oleg Nesterov wrote:
> > > > > 
> > > > > Could the freezer code be trying to freeze the idle thread as a result?
> > > > 
> > > > Yes. remove-the-likelypid-check-in-copy_process.patch make idle threads
> > > > visible to for_each_process/do_each_thread. Note also that idle threads
> > > > have ->mm != NULL. freezer, oom_kill, move_task_off_dead_cpu, etc, should
> > > > not see idle threads, but they do now.
> > > 
> > > Well, I think this is a good enough reason for asking Andrew to drop this
> > > patch.
> > 
> > Or we change the freezer to skip pid==0 tasks.
> 
> I strongly disagree. In that case we should audit all users of for_each_process.

Just in case, this is not the only problem with this patch. The things like
attach_pid(PIDTYPE_PGID/PIDTYPE_SID) or add_parent() are wrong for the idle
threads unless the parent is "swapper". But this is not always so.

Oleg.

Re: [linux-pm] 2.6.21-rc4-mm1: freezing of processes broken

[sukadev]

Andrew Morton [akpm@linux-foundation.org] wrote:
| On Wed, 21 Mar 2007 19:55:34 +0300 Oleg Nesterov <oleg@tv-sign.ru> wrote:
| 
| > On 03/21, Andrew Morton wrote:
| > >
| > > On Wed, 21 Mar 2007 15:45:16 +0100 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
| > > 
| > > > On Wednesday, 21 March 2007 15:23, Oleg Nesterov wrote:
| > > > > > 
| > > > > > Could the freezer code be trying to freeze the idle thread as a result?
| > > > > 
| > > > > Yes. remove-the-likelypid-check-in-copy_process.patch make idle threads
| > > > > visible to for_each_process/do_each_thread. Note also that idle threads
| > > > > have ->mm != NULL. freezer, oom_kill, move_task_off_dead_cpu, etc, should
| > > > > not see idle threads, but they do now.
| > > > 
| > > > Well, I think this is a good enough reason for asking Andrew to drop this
| > > > patch.
| > > 
| > > Or we change the freezer to skip pid==0 tasks.
| > 
| > I strongly disagree. In that case we should audit all users of for_each_process.
| > Imho, this change is too dangerous. Actually, I personally think it is very good
| > that idle threads are special and not visible, imho we should not change this.
| 
| OK, I'll tentatively drop remove-the-likelypid-check-in-copy_process.patch

Yes. Pls drop it for now. Here is the slightly modified 

	use-task_pgrp-task_session-in-copy_process.patch

This applies cleanly with remove-the-likelypid-check-in-copy_process.patch
removed.
---
From: Sukadev Bhattiprolu <sukadev@us.ibm.com>
Subject: [PATCH] Use task_pgrp() task_session() in copy_process().

Use task_pgrp() and task_session() in copy_process(), and
avoid find_pid() call when attaching the task to its process
group and session.

Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com>
---
 kernel/fork.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

Index: lx26-21-rc3-mm2/kernel/fork.c
===================================================================
--- lx26-21-rc3-mm2.orig/kernel/fork.c	2007-03-21 10:51:44.000000000 -0700
+++ lx26-21-rc3-mm2/kernel/fork.c	2007-03-21 10:53:48.000000000 -0700
@@ -1253,14 +1253,11 @@ static struct task_struct *copy_process(
 		tracehook_init_task(p);
 
 		if (thread_group_leader(p)) {
-			pid_t pgid = process_group(current);
-			pid_t sid = process_session(current);
-
 			p->signal->tty = current->signal->tty;
-			p->signal->pgrp = pgid;
+			p->signal->pgrp = process_group(current);
 			set_signal_session(p->signal, process_session(current));
-			attach_pid(p, PIDTYPE_PGID, find_pid(pgid));
-			attach_pid(p, PIDTYPE_SID, find_pid(sid));
+			attach_pid(p, PIDTYPE_PGID, task_pgrp(current));
+			attach_pid(p, PIDTYPE_SID, task_session(current));
 
 			list_add_tail_rcu(&p->tasks, &init_task.tasks);
 			__get_cpu_var(process_counts)++;

Re: 2.6.21-rc4-mm1: freezing of processes broken

[Eric W. Biederman]

While we are on the topic.  What became of the notion of zeroing the
session and process group values for the init_task?

Eric

Re: 2.6.21-rc4-mm1: freezing of processes broken

[sukadev]

Eric W. Biederman [ebiederm@xmission.com] wrote:
| 
| While we are on the topic.  What became of the notion of zeroing the
| session and process group values for the init_task?

I just sent that patch a few minutes ago.

| 
| Eric