From: Segher Boessenkool <segher@kernel.crashing.org>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: ravi.bangoria@linux.ibm.com, sxwjean@me.com,
Xiongwei Song <sxwjean@gmail.com>,
aneesh.kumar@linux.ibm.com, oleg@redhat.com, npiggin@gmail.com,
linux-kernel@vger.kernel.org, peterx@redhat.com,
paulus@samba.org, efremov@linux.com, akpm@linux-foundation.org,
linuxppc-dev@lists.ozlabs.org, sandipan@linux.ibm.com
Subject: Re: [RFC PATCH 1/4] powerpc: Optimize register usage for esr register
Date: Fri, 6 Aug 2021 09:26:43 -0500 [thread overview]
Message-ID: <20210806142643.GU1583@gate.crashing.org> (raw)
In-Reply-To: <874kc3njxh.fsf@mpe.ellerman.id.au>
On Fri, Aug 06, 2021 at 04:53:14PM +1000, Michael Ellerman wrote:
> But I'm not sure about the use of anonymous unions in UAPI headers. Old
> compilers don't support them, so there's a risk of breakage.
More precisely, it exists only since C11, so even with all not-so-ancient
compilers it will not work if the user uses (say) -std=c99, which still
is popular.
> I'd rather we didn't touch the uapi version.
Yeah.
> > - err = ___do_page_fault(regs, regs->dar, regs->dsisr);
> > + if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
> > + err = ___do_page_fault(regs, regs->dar, regs->esr);
> > + else
> > + err = ___do_page_fault(regs, regs->dar, regs->dsisr);
>
> As Christophe said, I don't thinks this is an improvement.
>
> It makes the code less readable. If anyone is confused about what is
> passed to ___do_page_fault() they can either read the comment above it,
> or look at the definition of pt_regs to see that esr and dsisr share
> storage.
Esp. since the affected platforms are legacy, yup.
Segher
WARNING: multiple messages have this Message-ID (diff)
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: sxwjean@me.com, linuxppc-dev@lists.ozlabs.org,
ravi.bangoria@linux.ibm.com, Xiongwei Song <sxwjean@gmail.com>,
oleg@redhat.com, npiggin@gmail.com, linux-kernel@vger.kernel.org,
efremov@linux.com, paulus@samba.org, aneesh.kumar@linux.ibm.com,
peterx@redhat.com, akpm@linux-foundation.org,
sandipan@linux.ibm.com
Subject: Re: [RFC PATCH 1/4] powerpc: Optimize register usage for esr register
Date: Fri, 6 Aug 2021 09:26:43 -0500 [thread overview]
Message-ID: <20210806142643.GU1583@gate.crashing.org> (raw)
In-Reply-To: <874kc3njxh.fsf@mpe.ellerman.id.au>
On Fri, Aug 06, 2021 at 04:53:14PM +1000, Michael Ellerman wrote:
> But I'm not sure about the use of anonymous unions in UAPI headers. Old
> compilers don't support them, so there's a risk of breakage.
More precisely, it exists only since C11, so even with all not-so-ancient
compilers it will not work if the user uses (say) -std=c99, which still
is popular.
> I'd rather we didn't touch the uapi version.
Yeah.
> > - err = ___do_page_fault(regs, regs->dar, regs->dsisr);
> > + if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
> > + err = ___do_page_fault(regs, regs->dar, regs->esr);
> > + else
> > + err = ___do_page_fault(regs, regs->dar, regs->dsisr);
>
> As Christophe said, I don't thinks this is an improvement.
>
> It makes the code less readable. If anyone is confused about what is
> passed to ___do_page_fault() they can either read the comment above it,
> or look at the definition of pt_regs to see that esr and dsisr share
> storage.
Esp. since the affected platforms are legacy, yup.
Segher
next prev parent reply other threads:[~2021-08-06 14:31 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-26 14:30 [RFC PATCH 1/4] powerpc: Optimize register usage for esr register sxwjean
2021-07-26 14:30 ` sxwjean
2021-07-26 14:30 ` [RFC PATCH 2/4] powerpc/64e: Get esr offset with _ESR macro sxwjean
2021-07-26 14:30 ` sxwjean
2021-07-26 14:30 ` [RFC PATCH 3/4] powerpc: Optimize register usage for dear register sxwjean
2021-07-26 14:30 ` sxwjean
2021-08-05 10:09 ` Christophe Leroy
2021-08-05 10:09 ` Christophe Leroy
2021-08-06 3:16 ` Xiongwei Song
2021-08-06 3:16 ` Xiongwei Song
2021-07-26 14:30 ` [RFC PATCH 4/4] powerpc/64e: Get dear offset with _DEAR macro sxwjean
2021-07-26 14:30 ` sxwjean
2021-08-05 10:06 ` [RFC PATCH 1/4] powerpc: Optimize register usage for esr register Christophe Leroy
2021-08-05 10:06 ` Christophe Leroy
2021-08-06 3:16 ` Xiongwei Song
2021-08-06 3:16 ` Xiongwei Song
2021-08-06 7:32 ` Christophe Leroy
2021-08-06 7:32 ` Christophe Leroy
2021-08-06 13:22 ` Xiongwei Song
2021-08-06 13:22 ` Xiongwei Song
2021-08-06 6:53 ` Michael Ellerman
2021-08-06 6:53 ` Michael Ellerman
2021-08-06 13:14 ` Xiongwei Song
2021-08-06 13:14 ` Xiongwei Song
2021-08-06 14:26 ` Segher Boessenkool [this message]
2021-08-06 14:26 ` Segher Boessenkool
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=20210806142643.GU1583@gate.crashing.org \
--to=segher@kernel.crashing.org \
--cc=akpm@linux-foundation.org \
--cc=aneesh.kumar@linux.ibm.com \
--cc=efremov@linux.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=npiggin@gmail.com \
--cc=oleg@redhat.com \
--cc=paulus@samba.org \
--cc=peterx@redhat.com \
--cc=ravi.bangoria@linux.ibm.com \
--cc=sandipan@linux.ibm.com \
--cc=sxwjean@gmail.com \
--cc=sxwjean@me.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.