Hi,
The first patch in the series separates the tasks freezer from the PM code and

moves it to kernel/freezer.c .  Some cleanups of freezer.c proposed by Gautham

will be sent in a separate patch.
The second one moves all of the freezer-specific per-task flags to a separate

filed of task_struct and defines functions for manipulating them with the help

of set_bit() and friends.  I think I have addressed all of the outstanding

issues with it.
The patches are against 2.6.21-rc7-mm2 with the Gautham's patch from

http://lkml.org/lkml/2007/4/26/250 applied.
Greetings,

Rafael
--

If you don't have the time to read,

you don't have the time or the tools to write.

		- Stephen King
-

From: Rafael J. Wysocki <rjw@sisk.pl>
Move all of the freezer-related flags to a separate field in task_struct and

introduce functions to operate them using set_bit() etc.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Acked-by: Pavel Machek <pavel@ucw.cz>

---

 Documentation/power/kernel_threads.txt |    2 -

 Documentation/power/swsusp.txt         |    4 +-

 arch/i386/kernel/apm.c                 |    2 -

 drivers/block/loop.c                   |    2 -

 drivers/char/apm-emulation.c           |    6 +--

 drivers/ieee1394/ieee1394_core.c       |    2 -

 drivers/md/md.c                        |    2 -

 drivers/mmc/card/queue.c               |    3 +

 drivers/mtd/mtd_blkdevs.c              |    3 +

 drivers/scsi/libsas/sas_scsi_host.c    |    2 -

 drivers/scsi/scsi_error.c              |    2 -

 drivers/usb/storage/usb.c              |    2 -

 include/asm-arm/thread_info.h          |    2 -

 include/asm-blackfin/thread_info.h     |    2 -

 include/asm-frv/thread_info.h          |    2 -

 include/asm-i386/thread_info.h         |    2 -

 include/asm-ia64/thread_info.h         |    2 -

 include/asm-mips/thread_info.h         |    2 -

 include/asm-powerpc/thread_info.h      |    2 -

 include/asm-sh/thread_info.h           |    2 -

 include/asm-x86_64/thread_info.h       |    2 -

 include/linux/freezer.h                |   63 +++++++++++++++++++++++++++------

 include/linux/sched.h                  |    8 ++--

 kernel/fork.c                          |    5 ++

 kernel/freezer.c                       |   10 ++---

 kernel/kthread.c                       |    3 +

 kernel/rcutorture.c                    |    4 +-

 kernel/sched.c                         |    2 -

 kernel/softirq.c                       |    2 -

 kernel/softlockup.c                    |    2 -

 kernel/workqueue.c                     |    2 -

 31 files changed, 90 insertions(+), 61 deletions(-)
Index: linux-2.6.21-rc7-mm2/include/linux/sched.h

==========================================================...
[ message continues ]

This patch

* Provides an interface to selectively freeze the system for different events.

* Allows tasks to exempt themselves or other tasks from specific freeze

  events.

* Allow nesting of freezer calls. For eg:
	freeze_processes(EVENT_A);

	/* Do something with respect to event A */

	.

	.

	.

	freeze_processes(EVENT_B);

	/* Do something with respect to event B */

	.

	.

	.

	thaw_processes(EVENT_B);

	.

	.

	.

	thaw_processes(EVENT_B);
This type of behaviour would be required when cpu hotplug would start

using the process freezer, where EVENT_A would be SUSPEND and EVENT_B

would be HOTPLUG_CPU.
This patch applies on the top of 2.6.21-rc7-mm2 + Rafael's freezer

changes from http://lkml.org/lkml/2007/4/27/302.
Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>

---

 arch/i386/kernel/apm.c              |    2 -

 drivers/block/loop.c                |    2 -

 drivers/char/apm-emulation.c        |    6 +--

 drivers/ieee1394/ieee1394_core.c    |    2 -

 drivers/md/md.c                     |    2 -

 drivers/mmc/card/queue.c            |    2 -

 drivers/mtd/mtd_blkdevs.c           |    2 -

 drivers/scsi/libsas/sas_scsi_host.c |    2 -

 drivers/scsi/scsi_error.c           |    2 -

 drivers/usb/storage/usb.c           |    2 -

 include/linux/freezer.h             |   44 +++++++++++++++++++-----

 kernel/freezer.c                    |   64 ++++++++++++++++++++++++++++++------

 kernel/kprobes.c                    |    4 +-

 kernel/kthread.c                    |    2 -

 kernel/power/disk.c                 |    4 +-

 kernel/power/main.c                 |    8 ++--

 kernel/power/user.c                 |    6 +--

 kernel/rcutorture.c                 |    4 +-

 kernel/sched.c                      |    2 -

 kernel/softirq.c                    |    2 -

 kernel/softlockup.c                 |    2 -

 kernel/workqueue.c                  |    2 -

 22 files changed, 119 insertions(+), 49 deletions(-)
Index: linux-2.6.21-rc7/include/linux/freezer.h

===============================...
[ message continues ]

Hi,
Sorry for the delay.
I, personally, would introduce
static inline void freezer_exempt_event(struct task_struct *p,

				  unsigned long freeze_event_mask)

{

	atomic_set_mask(freeze_event_mask, &p->freezer_flags);

}
and then
static inline void freezer_exempt(struct task_struct *p)

{

	freezer_exempt_event(p, FE_ALL);

}
The patch would be shorter. ;-)
[In that case I'd probably rename freezer_should_exempt() to
I would do
Hmm, I wouldn't use the WARN_ON() here.  There's nothing wrong in calling

this twice in a row as long as we do the sanity checking.  There's even one

-

I was wondering if the statement

	 if (process_frozen_event_mask(p) & ~current_freezer_event)

	 	return 0;
would be readable in the first place! 
Right! So we would need one more label. How about the following?
	mutex_lock(&freezer_mutex);

	/* check if already frozen for the event */

	if (system_frozen_event_mask & freeze_event)

		goto out_frozen;

		.

		.

		.
out_frozen:

	BUG_ON(in_atomic());

out:

	current_freezer_event = 0;

	mutex_unlock(&freezer_mutex);

	return nr_unfrozen;
Well, yes. But I put the warn on from the perspective of someone trying

to thaw_processes for the event for which they have not frozen. I hadn't

thought about a double thaw. Will rethink.
Thanks for the Review.

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

-

I think I'll duck this.  We have more than enough kthread/freezer/etc work

queued for 2.6.22.  Let's please for now concentrate on reviewing and

testing the existing changes.  Once that has all landed and settled in,

let's start thinking about the next round.
-

Sure, not a problem :-)
I just wanted to get a sense, if this was a right way to do it.

Besides, I am plannning to send it again once I port the cpuhotplug
Ok.
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!"

-

unsigned long freezer_flags; ??
Else it throws the following warnings.
include/linux/freezer.h: In function `frozen':

include/linux/freezer.h:22: warning: passing arg 2 of

`constant_test_bit' from incompatible pointer type

include/linux/freezer.h:22: warning: passing arg 2 of

`variable_test_bit' from incompatible pointer type

include/linux/freezer.h: In function `set_frozen_flag':

include/linux/freezer.h:27: warning: passing arg 2 of `set_bit' from

incompatible pointer type

include/linux/freezer.h: In function `clear_frozen_flag':

include/linux/freezer.h:32: warning: passing arg 2 of `clear_bit' from

incompatible pointer type

include/linux/freezer.h: In function `freezing':

include/linux/freezer.h:40: warning: passing arg 2 of

`constant_test_bit' from incompatible pointer type

include/linux/freezer.h:40: warning: passing arg 2 of

`variable_test_bit' from incompatible pointer type

include/linux/freezer.h: In function `freeze':

include/linux/freezer.h:48: warning: passing arg 2 of `set_bit' from

incompatible pointer type

include/linux/freezer.h: In function `clear_freeze_flag':

include/linux/freezer.h:56: warning: passing arg 2 of `clear_bit' from

incompatible pointer type

include/linux/freezer.h: In function `freezer_should_exempt':

include/linux/freezer.h:64: warning: passing arg 2 of

`constant_test_bit' from incompatible pointer type

include/linux/freezer.h:64: warning: passing arg 2 of

`variable_test_bit' from incompatible pointer type

include/linux/freezer.h: In function `freezer_exempt':

include/linux/freezer.h:72: warning: passing arg 2 of `set_bit' from

incompatible pointer type

include/linux/freezer.h: In function `freezer_do_not_count':

include/linux/freezer.h:132: warning: passing arg 2 of `set_bit' from

incompatible pointer type

include/linux/freezer.h: In function `freezer_count':

include/linux/freezer.h:142: warning: passing arg 2 of `clear_bit' from

incompatible pointer type

include/linux/freezer.h: In function `freezer_should_skip':

include/linux/freezer....
[ message continues ]

Greetings,

Rafael

-

Still, on x86_64, for example, we'll waste 4 bytes by using 'unsigned long'

here, which I wouldn't like to do.
Is it acceptable to use explicit type casting in set_bit() and friends?
Rafael

-

I'm afraid set_bit & friends are only defined on unsigned long. You

can cast, but you may run into some nasty surprise... like damaging

surrounding bytes on 64-bit architectures != x86_64.
--

(english) http://www.livejournal.com/~pavelmachek

(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

-

Let's use unsigned long, then, but I'm not really happy with that.
---

From: Rafael J. Wysocki <rjw@sisk.pl>
Move all of the freezer-related flags to a separate field in task_struct and

introduce functions to operate them using set_bit() etc.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

---

 Documentation/power/kernel_threads.txt |    2 -

 Documentation/power/swsusp.txt         |    4 +-

 arch/i386/kernel/apm.c                 |    2 -

 drivers/block/loop.c                   |    2 -

 drivers/char/apm-emulation.c           |    6 +--

 drivers/ieee1394/ieee1394_core.c       |    2 -

 drivers/md/md.c                        |    2 -

 drivers/mmc/card/queue.c               |    3 +

 drivers/mtd/mtd_blkdevs.c              |    3 +

 drivers/scsi/libsas/sas_scsi_host.c    |    2 -

 drivers/scsi/scsi_error.c              |    2 -

 drivers/usb/storage/usb.c              |    2 -

 include/asm-arm/thread_info.h          |    2 -

 include/asm-blackfin/thread_info.h     |    2 -

 include/asm-frv/thread_info.h          |    2 -

 include/asm-i386/thread_info.h         |    2 -

 include/asm-ia64/thread_info.h         |    2 -

 include/asm-mips/thread_info.h         |    2 -

 include/asm-powerpc/thread_info.h      |    2 -

 include/asm-sh/thread_info.h           |    2 -

 include/asm-x86_64/thread_info.h       |    2 -

 include/linux/freezer.h                |   63 +++++++++++++++++++++++++++------

 include/linux/sched.h                  |    8 ++--

 kernel/fork.c                          |    5 ++

 kernel/freezer.c                       |   10 ++---

 kernel/kthread.c                       |    3 +

 kernel/rcutorture.c                    |    4 +-

 kernel/sched.c                         |    2 -

 kernel/softirq.c                       |    2 -

 kernel/softlockup.c                    |    2 -

 kernel/workqueue.c                     |    2 -

 31 files changed, 90 insertions(+), 61 deletions(-)
Index: linux-2.6.21-rc7-mm2/include/linux/sched.h

========================...
[ message continues ]

Ask the architecture people, but comments like
/data/l/linux/include/linux/agpgart.h:  long access_flags;      /* long req'd for set_bit --RR */
Make me think long is needed.
 * bitmaps provide bit arrays that consume one or more unsigned

 * longs.  The bitmap interface and available operations are listed

 * here, in bitmap.h
...I'm afraid we need ulongs.

								Pavel
--

(english) http://www.livejournal.com/~pavelmachek

(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

-

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

-

Or maybe we have it already?
	Sam

-

That means something else. clear_freeze_flag() just clears the

TFF_FREEZE flag and not necessarily everything.
Besides, this is the only place where we would need to clear all the bits

of p->freezer_flags. Not sure if having a static inline helper function 
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!"

-

From: Rafael J. Wysocki <rjw@sisk.pl>
Now that the freezer is used by kprobes, it is no longer a PM-specific piece of

code.  Move the freezer code out of kernel/power and introduce the

CONFIG_FREEZER option that will be chosen automatically if PM or KPROBES is

set.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Acked-by: Pavel Machek <pavel@ucw.cz>

---

 arch/arm/Kconfig         |    5

 arch/avr32/Kconfig.debug |    5

 arch/blackfin/Kconfig    |    5

 arch/frv/Kconfig         |    5

 arch/i386/Kconfig        |    5

 arch/ia64/Kconfig        |    5

 arch/mips/Kconfig        |    5

 arch/powerpc/Kconfig     |    5

 arch/ppc/Kconfig         |    5

 arch/s390/Kconfig        |    5

 arch/sh/Kconfig          |    5

 arch/sparc64/Kconfig     |    5

 arch/x86_64/Kconfig      |    8 +

 include/linux/freezer.h  |    2

 kernel/Makefile          |    1

 kernel/freezer.c         |  236 +++++++++++++++++++++++++++++++++++++++++++++++

 kernel/kprobes.c         |    2

 kernel/power/Makefile    |    2

 kernel/power/process.c   |  236 -----------------------------------------------

 19 files changed, 308 insertions(+), 239 deletions(-)
Index: linux-2.6.21-rc7-mm2/arch/x86_64/Kconfig

===================================================================

--- linux-2.6.21-rc7-mm2.orig/arch/x86_64/Kconfig	2007-04-27 01:22:33.000000000 +0200

+++ linux-2.6.21-rc7-mm2/arch/x86_64/Kconfig	2007-04-27 01:33:22.000000000 +0200

@@ -703,6 +703,14 @@ config GENERIC_PENDING_IRQ

 	depends on GENERIC_HARDIRQS && SMP

 	default y
+#

+# Use the tasks freezer

+#

+config FREEZER

+	bool

+	default y

+	depends on PM || KPROBES

+

 menu "Power management options"
 source kernel/power/Kconfig

Index: linux-2.6.21-rc7-mm2/arch/avr32/Kconfig.debug

===================================================================

--- linux-2.6.21-rc7-mm2.orig/arch/avr32/Kconfig.debug	2007-04-27 01:22:33.000000000 +0200

+++ linux-2.6.21-rc7-mm2/arch/avr32/Kconfig.debug	2007-04-27 0...
[ message continues ]

Shouldn't PM and KPROBES "select" FREEZER if they want it?  That way its

easier for other future users of FREEZER to do so without having to

change the Kconfig here (and the other 23000 Kconfigs).
    J

-

Makes sense.  Please have a look at the updated patch below.
Sam, does this one look better to you?
---

From: Rafael J. Wysocki <rjw@sisk.pl>
Now that the freezer is used by kprobes, it is no longer a PM-specific piece of

code.  Move the freezer code out of kernel/power and introduce the

CONFIG_FREEZER option that will be chosen automatically if PM or KPROBES is

set.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

---

 arch/arm/Kconfig         |    4

 arch/avr32/Kconfig       |    4

 arch/avr32/Kconfig.debug |    1

 arch/blackfin/Kconfig    |    4

 arch/frv/Kconfig         |    4

 arch/i386/Kconfig        |    5

 arch/ia64/Kconfig        |    5

 arch/mips/Kconfig        |    4

 arch/powerpc/Kconfig     |    5

 arch/ppc/Kconfig         |    4

 arch/s390/Kconfig        |    5

 arch/sh/Kconfig          |    4

 arch/sparc64/Kconfig     |    5

 arch/x86_64/Kconfig      |    5

 include/linux/freezer.h  |    2

 kernel/Makefile          |    1

 kernel/freezer.c         |  236 +++++++++++++++++++++++++++++++++++++++++++++++

 kernel/kprobes.c         |    2

 kernel/power/Kconfig     |    1

 kernel/power/Makefile    |    2

 kernel/power/process.c   |  236 -----------------------------------------------

 21 files changed, 300 insertions(+), 239 deletions(-)
Index: linux-2.6.21-rc7-mm2/arch/x86_64/Kconfig

===================================================================

--- linux-2.6.21-rc7-mm2.orig/arch/x86_64/Kconfig	2007-04-27 21:41:05.000000000 +0200

+++ linux-2.6.21-rc7-mm2/arch/x86_64/Kconfig	2007-04-27 21:56:01.000000000 +0200

@@ -129,6 +129,10 @@ config ARCH_HAS_ILOG2_U64

 	bool

 	default n
+config FREEZER

+	bool

+	default n

+

 source "init/Kconfig"
@@ -791,6 +795,7 @@ source "arch/x86_64/oprofile/Kconfig"

 config KPROBES

 	bool "Kprobes (EXPERIMENTAL)"

 	depends on KALLSYMS && EXPERIMENTAL && MODULES

+	select FREEZER

 	help

 	  Kprobes allows you to trap at almost any kernel address and

 	  execute ...
[ message continues ]

If freezer.c is in kernel/, then shouldn't the corresponding config var

be in a non-arch Kconfig file?
    J

-

Well, I though it would look strange.  Still, I can do that, of course:
---

From: Rafael J. Wysocki <rjw@sisk.pl>
Now that the freezer is used by kprobes, it is no longer a PM-specific piece of

code.  Move the freezer code out of kernel/power and introduce the

CONFIG_FREEZER option that will be chosen automatically if PM or KPROBES is set.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

---

 arch/arm/Kconfig         |    2

 arch/avr32/Kconfig       |    2

 arch/avr32/Kconfig.debug |    1

 arch/blackfin/Kconfig    |    2

 arch/frv/Kconfig         |    2

 arch/i386/Kconfig        |    3

 arch/ia64/Kconfig        |    3

 arch/mips/Kconfig        |    2

 arch/powerpc/Kconfig     |    3

 arch/ppc/Kconfig         |    2

 arch/s390/Kconfig        |    3

 arch/sh/Kconfig          |    2

 arch/sparc64/Kconfig     |    3

 arch/x86_64/Kconfig      |    3

 include/linux/freezer.h  |    2

 kernel/Kconfig.freezer   |    5

 kernel/Makefile          |    1

 kernel/freezer.c         |  236 +++++++++++++++++++++++++++++++++++++++++++++++

 kernel/kprobes.c         |    2

 kernel/power/Kconfig     |    1

 kernel/power/Makefile    |    2

 kernel/power/process.c   |  236 -----------------------------------------------

 22 files changed, 279 insertions(+), 239 deletions(-)
Index: linux-2.6.21-rc7-mm2/arch/x86_64/Kconfig

===================================================================

--- linux-2.6.21-rc7-mm2.orig/arch/x86_64/Kconfig	2007-04-27 21:41:05.000000000 +0200

+++ linux-2.6.21-rc7-mm2/arch/x86_64/Kconfig	2007-04-27 23:20:43.000000000 +0200

@@ -703,6 +703,8 @@ config GENERIC_PENDING_IRQ

 	depends on GENERIC_HARDIRQS && SMP

 	default y
+source "kernel/Kconfig.freezer"

+

 menu "Power management options"
 source kernel/power/Kconfig

@@ -791,6 +793,7 @@ source "arch/x86_64/oprofile/Kconfig"

 config KPROBES

 	bool "Kprobes (EXPERIMENTAL)"

 	depends on KALLSYMS && EXPERIMENTAL && MODULES

+	select FREEZER

 	help

 	 ...
[ message continues ]

It would have been much better to have a single kernel/Kconfig

so you avoided all the arch specific source lines.

But thats not this patch and can come later.
So I'm OK with this one.
-

Could we have this in a generic Kconfig file instead?

IIRC the depends on line may refer to unknown options

without spitting out warnings.
	Sam

-