mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: Alastair D'Silva <alastair@au1.ibm.com>, alastair@d-silva.org
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Christophe Leroy <christophe.leroy@c-s.fr>,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>,
	Michal Hocko <mhocko@suse.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>,
	Paul Mackerras <paulus@samba.org>,
	Mike Rapoport <rppt@linux.vnet.ibm.com>
Subject: Re: [PATCH v2]  arch/powerpc: Rework local_paca to avoid LTO warnings
Date: Tue, 26 Mar 2019 15:58:43 +1000	[thread overview]
Message-ID: <1553579424.0r39b9otz6.astroid@bobo.none> (raw)
In-Reply-To: <20190314023125.10076-1-alastair@au1.ibm.com>

Alastair D'Silva's on March 14, 2019 12:31 pm:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> When building an LTO kernel, the existing code generates warnings:
>     ./arch/powerpc/include/asm/paca.h:37:30: warning: register of
>         ‘local_paca’ used for multiple global register variables
>      register struct paca_struct *local_paca asm("r13");
>                                   ^
>     ./arch/powerpc/include/asm/paca.h:37:30: note: conflicts with
>         ‘local_paca’

Isn't this a bogus warning? It doesn't look like there's a way to 
define it any other way.

> 
> This patch reworks local_paca into an inline getter & setter function,
> which addresses the warning.
> 
> Changelog:
> V2
>   - Address whitespace issues
>   - keep new implementation close to where the old implementation was
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>  arch/powerpc/include/asm/paca.h | 37 +++++++++++++++++++++++++--------
>  arch/powerpc/kernel/paca.c      |  2 +-
>  2 files changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
> index e843bc5d1a0f..2fa0b43357c9 100644
> --- a/arch/powerpc/include/asm/paca.h
> +++ b/arch/powerpc/include/asm/paca.h
> @@ -34,19 +34,38 @@
>  #include <asm/cpuidle.h>
>  #include <asm/atomic.h>
>  
> -register struct paca_struct *local_paca asm("r13");
> -
>  #if defined(CONFIG_DEBUG_PREEMPT) && defined(CONFIG_SMP)
>  extern unsigned int debug_smp_processor_id(void); /* from linux/smp.h */
> -/*
> - * Add standard checks that preemption cannot occur when using get_paca():
> - * otherwise the paca_struct it points to may be the wrong one just after.
> - */
> -#define get_paca()	((void) debug_smp_processor_id(), local_paca)
> -#else
> -#define get_paca()	local_paca
>  #endif
>  
> +static inline struct paca_struct *get_paca_no_preempt_check(void)
> +{
> +	register struct paca_struct *paca asm("r13");
> +
> +	return paca;
> +}

Problem is it now changes the global register variable to a local 
register variable. The compiler would presumably be within its rights
to "cache" that return value or use another register for it, which
is not really what we want.

Thanks,
Nick


  parent reply	other threads:[~2019-03-26  5:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-13  3:42 [PATCH 1/1] " Alastair D'Silva
2019-03-13  9:06 ` Christophe Leroy
2019-03-14  1:39   ` Alastair D'Silva
2019-03-13 23:54 ` Daniel Axtens
2019-03-14  0:09   ` Andrew Donnellan
2019-03-14  3:09   ` Alastair D'Silva
2019-03-14  2:31 ` [PATCH v2] " Alastair D'Silva
2019-03-14  5:46   ` Christophe Leroy
2019-03-26  5:58   ` Nicholas Piggin [this message]
2019-03-27  4:37     ` Alastair D'Silva
2019-03-27  6:02       ` Nicholas Piggin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1553579424.0r39b9otz6.astroid@bobo.none \
    --to=npiggin@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=alastair@au1.ibm.com \
    --cc=alastair@d-silva.org \
    --cc=benh@kernel.crashing.org \
    --cc=christophe.leroy@c-s.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mahesh@linux.vnet.ibm.com \
    --cc=mhocko@suse.com \
    --cc=mpe@ellerman.id.au \
    --cc=naveen.n.rao@linux.vnet.ibm.com \
    --cc=paulus@samba.org \
    --cc=rppt@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®