linux-kernel - Re: [PATCH 1/8] Enhance process freezer interface for usage beyond software suspend

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070402135632.GC6739@ucw.cz>
Date:	Mon, 2 Apr 2007 13:56:32 +0000
From:	Pavel Machek <pavel@....cz>
To:	Gautham R Shenoy <ego@...ibm.com>
Cc:	akpm@...ux-foundation.org, paulmck@...ibm.com,
	torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org,
	vatsa@...ibm.com, Oleg Nesterov <oleg@...sign.ru>,
	"Rafael J. Wysocki" <rjw@...k.pl>, mingo@...e.hu,
	dipankar@...ibm.com, dino@...ibm.com,
	masami.hiramatsu.pt@...achi.com
Subject: Re: [PATCH 1/8] Enhance process freezer interface for usage beyond software suspend

Hi!

>  This patch provides an interface to extend the use of the process
>  freezer beyond Suspend.
> 
> The tasks can selectively mark themselves to be exempted from specific
> freeze events like SUSPEND /KPROBES/CPU_HOTPLUG.
> 
> This patch however, *does not* sort non freezable threads into
> different categories based on the freeze events. Thus all 
> tasks which were previously marked PF_NOFREEZE are now
> exempted from freezer using 
> 	freezer_exempt(FE_ALL);
> which means exempt from all kinds of freezes.
> 
> Signed-off-by: Gautham R Shenoy <ego@...ibm.com>
> Cc: Pavel Machek <pavel@....cz>
> Cc: Rafael J. Wysocki <rjw@...k.pl>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>

Actually no, I was not in cc. 

> +/* Per process freezer specific flags */
> +#define PF_FE_SUSPEND	0x00008000	/* This thread should not be frozen
> +					 * for suspend
> +					 */
> +
> +#define PF_FE_KPROBES	0x00000010	/* This thread should not be frozen
> +					 * for Kprobes
> +					 */

Just put the comment before the define for long comments?


> -#ifdef CONFIG_PM
> +#if defined(CONFIG_PM) || defined(CONFIG_HOTPLUG_CPU) || \
> +					defined(CONFIG_KPROBES)

Should we create CONFIG_FREEZER?

> Index: linux-2.6.21-rc5/kernel/softlockup.c
> ===================================================================
> --- linux-2.6.21-rc5.orig/kernel/softlockup.c
> +++ linux-2.6.21-rc5/kernel/softlockup.c
> @@ -96,7 +96,7 @@ static int watchdog(void * __bind_cpu)
>  	struct sched_param param = { .sched_priority = MAX_RT_PRIO-1 };
>  
>  	sched_setscheduler(current, SCHED_FIFO, &param);
> -	current->flags |= PF_NOFREEZE;
> +	freezer_exempt(FE_ALL);
>  
>  	/*
>  	 * Run briefly once per second to reset the softlockup timestamp.

Hmmm, I do not really like softlockup watchdog running during suspend.
Can we make this freezeable and make watchdog shut itself off while
suspending?

> Index: linux-2.6.21-rc5/kernel/rcutorture.c
> ===================================================================
> --- linux-2.6.21-rc5.orig/kernel/rcutorture.c
> +++ linux-2.6.21-rc5/kernel/rcutorture.c
> @@ -559,7 +559,7 @@ rcu_torture_fakewriter(void *arg)
>  
>  	VERBOSE_PRINTK_STRING("rcu_torture_fakewriter task started");
>  	set_user_nice(current, 19);
> -	current->flags |= PF_NOFREEZE;
> +	freezer_exempt(FE_ALL);


Fix rcutorture instead. It has no business running while suspending.

>  
>  	do {
>  		schedule_timeout_uninterruptible(1 + rcu_random(&rand)%10);
> @@ -590,7 +590,7 @@ rcu_torture_reader(void *arg)
>  
>  	VERBOSE_PRINTK_STRING("rcu_torture_reader task started");
>  	set_user_nice(current, 19);
> -	current->flags |= PF_NOFREEZE;
> +	freezer_exempt(FE_ALL);
>  

Same here.

Eventually, we should fix apm, too.

> Index: linux-2.6.21-rc5/init/do_mounts_initrd.c
> ===================================================================
> --- linux-2.6.21-rc5.orig/init/do_mounts_initrd.c
> +++ linux-2.6.21-rc5/init/do_mounts_initrd.c
> @@ -55,7 +55,7 @@ static void __init handle_initrd(void)
>  	sys_mount(".", "/", NULL, MS_MOVE, NULL);
>  	sys_chroot(".");
>  
> -	current->flags |= PF_NOFREEZE;
> +	freezer_exempt(FE_ALL);
>  	pid = kernel_thread(do_linuxrc, "/linuxrc", SIGCHLD);
>  	if (pid > 0) {
>  		while (pid != sys_wait4(-1, NULL, 0, NULL))

Does this mean we have userland /linuxrc running with PF_NOFREEZE?
That would be very bad...

> --- linux-2.6.21-rc5.orig/kernel/kprobes.c
> +++ linux-2.6.21-rc5/kernel/kprobes.c
> @@ -104,7 +104,7 @@ static int __kprobes check_safety(void)
>  {
>  	int ret = 0;
>  #if defined(CONFIG_PREEMPT) && defined(CONFIG_PM)

Eh? Why does kprobes code depend on config_pm?

						Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/