All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alastair D'Silva" <alastair@au1.ibm.com>
To: Daniel Axtens <dja@axtens.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Nicholas Piggin <npiggin@gmail.com>,
	Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>,
	"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>,
	Christophe Leroy <christophe.leroy@c-s.fr>,
	Michal Hocko <mhocko@suse.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Mike Rapoport <rppt@linux.vnet.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] arch/powerpc: Rework local_paca to avoid LTO warnings
Date: Thu, 14 Mar 2019 14:09:22 +1100	[thread overview]
Message-ID: <07a2633c1264db39d2ef91d6824c319f57027c26.camel@au1.ibm.com> (raw)
In-Reply-To: <87ef7atjnk.fsf@dja-thinkpad.axtens.net>

On Thu, 2019-03-14 at 10:54 +1100, Daniel Axtens wrote:
> "Alastair D'Silva" <alastair@au1.ibm.com> writes:
> 
> > 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’
> > 
> > This patch reworks local_paca into an inline getter & setter
> > function,
> > which addresses the warning.
> > 
> > Generated ASM from this patch is broadly similar (addresses have
> > changed and the compiler uses different GPRs in some places).
> 
> Ditto to Christophe's comment; I'd love to know how to build this so
> I
> can actually see the differences. Perhaps you could bundle up all the
> required changes and send it as a patch series with a cover letter
> explaining this?

The differences are visible in a normal build, but if you want to play
with LTO, see my comments to Christophe.

> 
> > +static inline struct paca_struct *get_paca_no_preempt_check(void)
> > +{
> > +	register struct paca_struct *paca asm("r13");
> > +	return paca;
> > +}
> 
> Isn't the convention to have the { on the same line as the function,
> or
> am I horrible mis-remembering things?
> 
You are :)

> Should these functions be __always_inline?
> 

Yes, they should, I'll add that to V3.

-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819


WARNING: multiple messages have this Message-ID (diff)
From: "Alastair D'Silva" <alastair@au1.ibm.com>
To: Daniel Axtens <dja@axtens.net>
Cc: Michal Hocko <mhocko@suse.com>,
	Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org, Nicholas Piggin <npiggin@gmail.com>,
	Mike Rapoport <rppt@linux.vnet.ibm.com>,
	Paul Mackerras <paulus@samba.org>,
	"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>,
	linuxppc-dev@lists.ozlabs.org,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 1/1] arch/powerpc: Rework local_paca to avoid LTO warnings
Date: Thu, 14 Mar 2019 14:09:22 +1100	[thread overview]
Message-ID: <07a2633c1264db39d2ef91d6824c319f57027c26.camel@au1.ibm.com> (raw)
In-Reply-To: <87ef7atjnk.fsf@dja-thinkpad.axtens.net>

On Thu, 2019-03-14 at 10:54 +1100, Daniel Axtens wrote:
> "Alastair D'Silva" <alastair@au1.ibm.com> writes:
> 
> > 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’
> > 
> > This patch reworks local_paca into an inline getter & setter
> > function,
> > which addresses the warning.
> > 
> > Generated ASM from this patch is broadly similar (addresses have
> > changed and the compiler uses different GPRs in some places).
> 
> Ditto to Christophe's comment; I'd love to know how to build this so
> I
> can actually see the differences. Perhaps you could bundle up all the
> required changes and send it as a patch series with a cover letter
> explaining this?

The differences are visible in a normal build, but if you want to play
with LTO, see my comments to Christophe.

> 
> > +static inline struct paca_struct *get_paca_no_preempt_check(void)
> > +{
> > +	register struct paca_struct *paca asm("r13");
> > +	return paca;
> > +}
> 
> Isn't the convention to have the { on the same line as the function,
> or
> am I horrible mis-remembering things?
> 
You are :)

> Should these functions be __always_inline?
> 

Yes, they should, I'll add that to V3.

-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819


  parent reply	other threads:[~2019-03-14  3:09 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-13  3:42 [PATCH 1/1] arch/powerpc: Rework local_paca to avoid LTO warnings Alastair D'Silva
2019-03-13  3:42 ` Alastair D'Silva
2019-03-13  9:06 ` Christophe Leroy
2019-03-13  9:06   ` Christophe Leroy
2019-03-14  1:39   ` Alastair D'Silva
2019-03-14  1:39     ` Alastair D'Silva
2019-03-13 23:54 ` Daniel Axtens
2019-03-13 23:54   ` Daniel Axtens
2019-03-14  0:09   ` Andrew Donnellan
2019-03-14  0:09     ` Andrew Donnellan
2019-03-14  3:09   ` Alastair D'Silva [this message]
2019-03-14  3:09     ` Alastair D'Silva
2019-03-14  2:31 ` [PATCH v2] " Alastair D'Silva
2019-03-14  2:31   ` Alastair D'Silva
2019-03-14  5:46   ` Christophe Leroy
2019-03-14  5:46     ` Christophe Leroy
2019-03-26  5:58   ` Nicholas Piggin
2019-03-26  5:58     ` Nicholas Piggin
2019-03-27  4:37     ` Alastair D'Silva
2019-03-27  4:37       ` Alastair D'Silva
2019-03-27  6:02       ` Nicholas Piggin
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=07a2633c1264db39d2ef91d6824c319f57027c26.camel@au1.ibm.com \
    --to=alastair@au1.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=christophe.leroy@c-s.fr \
    --cc=dja@axtens.net \
    --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=npiggin@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.