Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 9 Jun 2020 14:38:29 -0700
From:   Eric Biggers <ebiggers@kernel.org>
To:     Guenter Roeck <linux@roeck-us.net>
Cc:     Peter Zijlstra <peterz@infradead.org>, tglx@linutronix.de,
        frederic@kernel.org, linux-kernel@vger.kernel.org, x86@kernel.org,
        cai@lca.pw, mgorman@techsingularity.net
Subject: Re: [RFC][PATCH 7/7] sched: Replace rq::wake_list
Message-ID: <20200609213829.GA789@sol.localdomain>
References: <20200526161057.531933155@infradead.org>
 <20200526161908.129371594@infradead.org>
 <20200604141837.GA179816@roeck-us.net>
 <20200605002433.GA148196@sol.localdomain>
 <20200605074154.GB2750@hirez.programming.kicks-ass.net>
 <20200605161532.GD1373@sol.localdomain>
 <53318971-561c-b445-0408-530b3d3ba44e@roeck-us.net>
 <20200609202134.GA1105@sol.localdomain>
 <20200609212509.GA239889@roeck-us.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200609212509.GA239889@roeck-us.net>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Jun 09, 2020 at 02:25:09PM -0700, Guenter Roeck wrote:
> > 
> > Still occurring on Linus' tree.  This needs to be fixed.  (And not by removing
> > support for randstruct; that's not a "fix"...)
> > 
> 
> How about the hack below ?
> 
> Guenter
> 
> ---
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index c5d96e3e7fff..df1cbb04f9b3 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -629,6 +629,15 @@ struct wake_q_node {
>  	struct wake_q_node *next;
>  };
>  
> +/*
> + * Hack around assumption that wake_entry_type follows wake_entry even with
> + * CONFIG_GCC_PLUGIN_RANDSTRUCT=y.
> + */
> +struct _wake_entry {
> +	struct llist_node	wake_entry;
> +	unsigned int		wake_entry_type;
> +};
> +
>  struct task_struct {
>  #ifdef CONFIG_THREAD_INFO_IN_TASK
>  	/*
> @@ -653,8 +662,9 @@ struct task_struct {
>  	unsigned int			ptrace;
>  
>  #ifdef CONFIG_SMP
> -	struct llist_node		wake_entry;
> -	unsigned int			wake_entry_type;
> +	struct _wake_entry		_we;
> +#define wake_entry		_we.wake_entry
> +#define wake_entry_type		_we.wake_entry_type
>  	int				on_cpu;
>  #ifdef CONFIG_THREAD_INFO_IN_TASK
>  	/* Current CPU: */

Does the struct actually have to be named?  How about:

diff --git a/include/linux/sched.h b/include/linux/sched.h
index c5d96e3e7fff42..14ca25cda19150 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -653,8 +653,14 @@ struct task_struct {
 	unsigned int			ptrace;
 
 #ifdef CONFIG_SMP
-	struct llist_node		wake_entry;
-	unsigned int			wake_entry_type;
+	/*
+	 * wake_entry_type must follow wake_entry, even when
+	 * CONFIG_GCC_PLUGIN_RANDSTRUCT=y.
+	 */
+	struct {
+		struct llist_node	wake_entry;
+		unsigned int		wake_entry_type;
+	};
 	int				on_cpu;
 #ifdef CONFIG_THREAD_INFO_IN_TASK
 	/* Current CPU: */


However, it would be preferable to not rely on different structs sharing the
same field order, but rather write proper C code that uses the same struct
everywhere to encapsulate these 2 fields...

- Eric