2007-05-09 10:45:15

by Stefan Richter

[permalink] [raw]
Subject: Re: [patch 128/197] freezer: add try_to_freeze calls to all kernel threads

[email protected] wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> 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.

A few questions:

Does try_to_freeze()'s kerneldoc document that try_to_freeze() is a
no-op sometimes but should nevertheless be called for this and that
reason? (I don't know the entire patch series.)

Why add no-op-try_to_freeze() everywhere now, instead of adding it later
when it will actually be needed? (I.e. "in the future".)

Can we please have a future where no device driver has to care if and
when and how to freeze its threads?
--
Stefan Richter
-=====-=-=== -=-= -=--=
http://arcgraph.de/sr/


2007-05-09 10:50:17

by Pavel Machek

[permalink] [raw]
Subject: Re: [patch 128/197] freezer: add try_to_freeze calls to all kernel threads

Hi!

> > 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.
>
> A few questions:
>
> Does try_to_freeze()'s kerneldoc document that try_to_freeze() is a
> no-op sometimes but should nevertheless be called for this and that
> reason? (I don't know the entire patch series.)
>
> Why add no-op-try_to_freeze() everywhere now, instead of adding it later
> when it will actually be needed? (I.e. "in the future".)

It is needed later in the patch series... for kprobes, etc.

> Can we please have a future where no device driver has to care if and
> when and how to freeze its threads?

No. Freezing is useful for kprobes/cpu hotplug as well as suspend.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2007-05-09 12:07:25

by Stefan Richter

[permalink] [raw]
Subject: Re: [patch 128/197] freezer: add try_to_freeze calls to all kernel threads

Pavel Machek wrote:
>> > 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).
[...]
>> Why add no-op-try_to_freeze() everywhere now, instead of adding it later
>> when it will actually be needed? (I.e. "in the future".)
>
> It is needed later in the patch series... for kprobes, etc.

So does freezer_exempt() still do what its name says, or does it
freezer_exempt_but_not_always() now?
--
Stefan Richter
-=====-=-=== -=-= -=--=
http://arcgraph.de/sr/

Subject: Re: [patch 128/197] freezer: add try_to_freeze calls to all kernel threads

Hi Stefan,
On Wed, May 09, 2007 at 02:06:05PM +0200, Stefan Richter wrote:
> Pavel Machek wrote:
> >> > 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).
> [...]
> >> Why add no-op-try_to_freeze() everywhere now, instead of adding it later
> >> when it will actually be needed? (I.e. "in the future".)
> >
> > It is needed later in the patch series... for kprobes, etc.
>
> So does freezer_exempt() still do what its name says, or does it
> freezer_exempt_but_not_always() now?

freezer_exempt() as of now does what its name says. I.e, exempt the
thread from all kinds of freeze chills.

But with more subsystems using the process freezer, the exemption needs
to be event specific. There may be threads which should not be frozen
for say kprobes, should be frozen for cpu-hotplug. This selective
freezing is not yet available. But it will be soon...

> --
> Stefan Richter
> -=====-=-=== -=-= -=--=
> http://arcgraph.de/sr/

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!"

2007-05-09 13:21:58

by Stefan Richter

[permalink] [raw]
Subject: Re: [patch 128/197] freezer: add try_to_freeze calls to all kernel threads

Gautham R Shenoy wrote:
> freezer_exempt() as of now does what its name says. I.e, exempt the
> thread from all kinds of freeze chills.
>
> But with more subsystems using the process freezer, the exemption needs
> to be event specific. There may be threads which should not be frozen
> for say kprobes, should be frozen for cpu-hotplug. This selective
> freezing is not yet available. But it will be soon...

Thanks for the (necessary!) clarification.
Let me point out that the usual process would be to replace

freezer_exempt(current);
for (;;) {
...;
by
freezer_exempt_for_io(current);
for (;;) {
try_to_freeze();
...;

when or after freezer_exempt_for_io was implemented.

But as it was submitted now, we are temporarily left with

freezer_exempt(current);
for (;;) {
try_to_freeze(); /* useless irritating no-op */
...;

without any benefit. (And this explanatory comment ^^^ wasn't even
added; we only have the git log as explanation.)

As subsystem maintainer I have to trust now that "soon" actually means
"soon" and not "RSN"; otherwise my responsibility would be to send a NAK.
--
Stefan Richter
-=====-=-=== -=-= -=--=
http://arcgraph.de/sr/

Subject: Re: [patch 128/197] freezer: add try_to_freeze calls to all kernel threads

On Wed, May 09, 2007 at 03:20:47PM +0200, Stefan Richter wrote:
> Gautham R Shenoy wrote:
> > freezer_exempt() as of now does what its name says. I.e, exempt the
> > thread from all kinds of freeze chills.
> >
> > But with more subsystems using the process freezer, the exemption needs
> > to be event specific. There may be threads which should not be frozen
> > for say kprobes, should be frozen for cpu-hotplug. This selective
> > freezing is not yet available. But it will be soon...
>
> Thanks for the (necessary!) clarification.
> Let me point out that the usual process would be to replace
>
> freezer_exempt(current);
> for (;;) {
> ...;
> by
> freezer_exempt_for_io(current);
> for (;;) {
> try_to_freeze();
> ...;
>
> when or after freezer_exempt_for_io was implemented.
>

Well, a couple of RFC's have already been sent with this regard.
Most of these recent freezer changes resulted due to the discussions
that took place over these RFC's.

This was the first attempt
http://lkml.org/lkml/2007/2/14/106

and a more recent one
http://lkml.org/lkml/2007/4/2/33

> But as it was submitted now, we are temporarily left with
>
> freezer_exempt(current);
> for (;;) {
> try_to_freeze(); /* useless irritating no-op */
> ...;
>
> without any benefit. (And this explanatory comment ^^^ wasn't even
> added; we only have the git log as explanation.)
>
> As subsystem maintainer I have to trust now that "soon" actually means
> "soon" and not "RSN"; otherwise my responsibility would be to send a NAK.

Soon actually does mean soon :-)

http://lkml.org/lkml/2007/4/27/616 was sent out recently.
I am working on the Rafael's suggestions.
The only thing holding these patches back is the fact that
quite an amount of patches on the freezer/kthread front has gone in
recently, which need more review and testing.

Will keep you posted on the freezer developments from now on.

> --
> Stefan Richter
> -=====-=-=== -=-= -=--=
> http://arcgraph.de/sr/

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!"

2007-05-09 18:18:16

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [patch 128/197] freezer: add try_to_freeze calls to all kernel threads

On Wednesday, 9 May 2007 17:29, Gautham R Shenoy wrote:
> On Wed, May 09, 2007 at 03:20:47PM +0200, Stefan Richter wrote:
> > Gautham R Shenoy wrote:
> > > freezer_exempt() as of now does what its name says. I.e, exempt the
> > > thread from all kinds of freeze chills.
> > >
> > > But with more subsystems using the process freezer, the exemption needs
> > > to be event specific. There may be threads which should not be frozen
> > > for say kprobes, should be frozen for cpu-hotplug. This selective
> > > freezing is not yet available. But it will be soon...
> >
> > Thanks for the (necessary!) clarification.
> > Let me point out that the usual process would be to replace
> >
> > freezer_exempt(current);
> > for (;;) {
> > ...;
> > by
> > freezer_exempt_for_io(current);
> > for (;;) {
> > try_to_freeze();
> > ...;
> >
> > when or after freezer_exempt_for_io was implemented.
> >
>
> Well, a couple of RFC's have already been sent with this regard.
> Most of these recent freezer changes resulted due to the discussions
> that took place over these RFC's.
>
> This was the first attempt
> http://lkml.org/lkml/2007/2/14/106
>
> and a more recent one
> http://lkml.org/lkml/2007/4/2/33
>
> > But as it was submitted now, we are temporarily left with
> >
> > freezer_exempt(current);
> > for (;;) {
> > try_to_freeze(); /* useless irritating no-op */
> > ...;
> >
> > without any benefit. (And this explanatory comment ^^^ wasn't even
> > added; we only have the git log as explanation.)

I feel I ought to explain why it's ended up in this state.

This is not a new patch. It's been in -mm since 2.6.21-rc2-mm1 (at least)
and we hoped to have the selective freezing of the system for different events
done before the 2.6.22 merging window would open. It didn't happen, mainly
for reasons that we couldn't control, and now we have some patches that
depend on this one and are more urgent, since they're needed to fix some bugs.

> > As subsystem maintainer I have to trust now that "soon" actually means
> > "soon" and not "RSN"; otherwise my responsibility would be to send a NAK.
>
> Soon actually does mean soon :-)
>
> http://lkml.org/lkml/2007/4/27/616 was sent out recently.
> I am working on the Rafael's suggestions.
> The only thing holding these patches back is the fact that
> quite an amount of patches on the freezer/kthread front has gone in
> recently, which need more review and testing.
>
> Will keep you posted on the freezer developments from now on.

Me too.

Greetings,
Rafael

2007-05-09 18:50:26

by Stefan Richter

[permalink] [raw]
Subject: Re: [patch 128/197] freezer: add try_to_freeze calls to all kernel threads

Rafael J. Wysocki wrote:
> On Wednesday, 9 May 2007 17:29, Gautham R Shenoy wrote:
>> On Wed, May 09, 2007 at 03:20:47PM +0200, Stefan Richter wrote:
>>> Let me point out that the usual process would be
[...to do it in one go....]
>> Well, a couple of RFC's have already been sent with this regard.
>> Most of these recent freezer changes resulted due to the discussions
>> that took place over these RFC's.
[...]
> I feel I ought to explain why it's ended up in this state.
>
> This is not a new patch. It's been in -mm since 2.6.21-rc2-mm1 (at least)
> and we hoped to have the selective freezing of the system for different events
> done before the 2.6.22 merging window would open. It didn't happen, mainly
> for reasons that we couldn't control, and now we have some patches that
> depend on this one and are more urgent, since they're needed to fix some bugs.

OK, thanks once more for the explanations.

[...]
>> Will keep you posted on the freezer developments from now on.
>
> Me too.

Well, the mistake on my part was that I didn't pull -mm regularly,
otherwise I would have automatically seen the change to the one kthread
I am interested in.

Thanks,
--
Stefan Richter
-=====-=-=== -=-= -=--=
http://arcgraph.de/sr/