linux-kernel - Re: [PATCH v3 6/6] freezer,sched: Rewrite core freezer logic

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: <YxR5Yauhd90WN/AY@gmail.com>
Date:   Sun, 4 Sep 2022 12:09:37 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     rjw@...ysocki.net, oleg@...hat.com, vincent.guittot@...aro.org,
        dietmar.eggemann@....com, rostedt@...dmis.org, mgorman@...e.de,
        ebiederm@...ssion.com, bigeasy@...utronix.de,
        Will Deacon <will@...nel.org>, linux-kernel@...r.kernel.org,
        tj@...nel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v3 6/6] freezer,sched: Rewrite core freezer logic


* Peter Zijlstra <peterz@...radead.org> wrote:

> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -81,25 +81,32 @@ struct task_group;
>   */
>  
>  /* Used in tsk->state: */
> -#define TASK_RUNNING			0x0000
> -#define TASK_INTERRUPTIBLE		0x0001
> -#define TASK_UNINTERRUPTIBLE		0x0002
> -#define __TASK_STOPPED			0x0004
> -#define __TASK_TRACED			0x0008
> +#define TASK_RUNNING			0x000000
> +#define TASK_INTERRUPTIBLE		0x000001
> +#define TASK_UNINTERRUPTIBLE		0x000002
> +#define __TASK_STOPPED			0x000004
> +#define __TASK_TRACED			0x000008
>  /* Used in tsk->exit_state: */
> -#define EXIT_DEAD			0x0010
> -#define EXIT_ZOMBIE			0x0020
> +#define EXIT_DEAD			0x000010
> +#define EXIT_ZOMBIE			0x000020
>  #define EXIT_TRACE			(EXIT_ZOMBIE | EXIT_DEAD)
>  /* Used in tsk->state again: */
> -#define TASK_PARKED			0x0040
> -#define TASK_DEAD			0x0080
> -#define TASK_WAKEKILL			0x0100
> -#define TASK_WAKING			0x0200
> -#define TASK_NOLOAD			0x0400
> -#define TASK_NEW			0x0800
> -/* RT specific auxilliary flag to mark RT lock waiters */
> -#define TASK_RTLOCK_WAIT		0x1000
> -#define TASK_STATE_MAX			0x2000
> +#define TASK_PARKED			0x000040
> +#define TASK_DEAD			0x000080
> +#define TASK_WAKEKILL			0x000100
> +#define TASK_WAKING			0x000200
> +#define TASK_NOLOAD			0x000400
> +#define TASK_NEW			0x000800
> +#define TASK_FREEZABLE			0x001000
> +#define __TASK_FREEZABLE_UNSAFE	       (0x002000 * IS_ENABLED(CONFIG_LOCKDEP))
> +#define TASK_FROZEN			0x004000
> +#define TASK_RTLOCK_WAIT		0x008000
> +#define TASK_STATE_MAX			0x010000

Patch ordering suggestion: would be really nice to first do the width 
adjustment as a preparatory patch, then the real changes. The mixing 
obscures what the patch is doing here, that we leave all bits before 
TASK_NEW unchanged, add in TASK_FREEZABLE, __TASK_FREEZABLE_UNSAFE & 
TASK_FROZEN to before TASK_RTLOCK_WAIT.

Btw., wouldn't it be better to just add the new bits right before 
TASK_STATE_MAX, and leave the existing ones unchanged? I don't think the 
order of TASK_RTLOCK_WAIT is relevant, right?

>  /* Convenience macros for the sake of set_current_state: */
>  #define TASK_KILLABLE			(TASK_WAKEKILL | TASK_UNINTERRUPTIBLE)
> @@ -1714,7 +1721,6 @@ extern struct pid *cad_pid;
>  #define PF_NPROC_EXCEEDED	0x00001000	/* set_user() noticed that RLIMIT_NPROC was exceeded */
>  #define PF_USED_MATH		0x00002000	/* If unset the fpu must be initialized before use */
>  #define PF_NOFREEZE		0x00008000	/* This thread should not be frozen */
> -#define PF_FROZEN		0x00010000	/* Frozen for system suspend */
>  #define PF_KSWAPD		0x00020000	/* I am kswapd */
>  #define PF_MEMALLOC_NOFS	0x00040000	/* All allocation requests will inherit GFP_NOFS */
>  #define PF_MEMALLOC_NOIO	0x00080000	/* All allocation requests will inherit GFP_NOIO */

yay.

BTW., we should probably mark/document all PF_ holes with a PF__RESERVED 
kind of scheme? Something simple, like:

   #define PF_NPROC_EXCEEDED	0x00001000	/* set_user() noticed that RLIMIT_NPROC was exceeded */
   #define PF_USED_MATH		0x00002000	/* If unset the fpu must be initialized before use */
 + #define PF__RESERVED_04000	0x00004000	/* Unused */
   #define PF_NOFREEZE		0x00008000	/* This thread should not be frozen */
 + #define PF__RESERVED_10000	0x00010000	/* Unused */
   #define PF_KSWAPD		0x00020000	/* I am kswapd */
   #define PF_MEMALLOC_NOFS	0x00040000	/* All allocation requests will inherit GFP_NOFS */
   #define PF_MEMALLOC_NOIO	0x00080000	/* All allocation requests will inherit GFP_NOIO */

?

Thanks,

	Ingo