From: "Nicholas Piggin" <npiggin@gmail•com>
To: "Benjamin Gray" <bgray@linux•ibm.com>,
"Christophe Leroy" <christophe.leroy@csgroup•eu>,
"linuxppc-dev@lists•ozlabs.org" <linuxppc-dev@lists•ozlabs.org>
Cc: "jniethe5@gmail•com" <jniethe5@gmail•com>,
"ajd@linux•ibm.com" <ajd@linux•ibm.com>,
"cmr@bluescreens•de" <cmr@bluescreens•de>
Subject: Re: [PATCH v9 5/7] powerpc/tlb: Add local flush for page given mm_struct and psize
Date: Mon, 07 Nov 2022 22:28:26 +1000 [thread overview]
Message-ID: <CO62AX0EBW9Y.2SCODZ31UQD4N@bobo> (raw)
In-Reply-To: <cd76d514a59088706721bbd3817b0ae664751acc.camel@linux.ibm.com>
On Mon Nov 7, 2022 at 4:58 PM AEST, Benjamin Gray wrote:
> On Thu, 2022-11-03 at 11:39 +1100, Benjamin Gray wrote:
> > On Wed, 2022-11-02 at 09:56 +0000, Christophe Leroy wrote:
> > > By the way, 'extern' keyword is pointless and deprecated for
> > > functions
> > > prototypes, please don't add new ones, even if other historical
> > > prototypes have one.
> >
> > This and the above commented parts match the style of the surrounding
> > implementations. For example,
> >
> > static inline void local_flush_tlb_mm(struct mm_struct *mm)
> > {
> > if (radix_enabled())
> > return radix__local_flush_tlb_mm(mm);
> > return hash__local_flush_tlb_mm(mm);
> > }
> >
> > I am not going to add code that is inconsistent with the surrounding
> > code. That just causes confusion later down the line when readers
> > wonder why this function is special compared to the others. If it
> > needs
> > to use modern style, then I would be happy to include a patch that
> > modernises the surrounding code first.
> >
> > Though why the hash__* functions are empty I'm not sure... If they
> > were
> > not implemented I would have expected a BUILD_BUG(). If they are
> > unnecessary due to hash itself, it's odd that they exist (as you
> > point
> > out for the new one).
>
> From what I can see in the history, the empty hash functions were
> originally introduced as 64-bit compatibility definitions for the hash
> MMU (which I guess doesn't require any action).
Yeah the hash MMU does hash PTE update and TLB flushing in the Linux pte
update APIs (which end up at hash__pte_update()). By the time Linux
calls flush_tlb_xxx(), the powerpc code had already done the necessary
TLB flushing.
> These empty functions
> were shuffled around, then eventually prefixed with hash__* to make way
> for the radix variants, which are hidden behind a generic
> 'local_flush_tlb_mm' etc. implementation as we see today. So basically,
> the empty hash__* functions no longer (never, really) served a purpose
> once the generic wrapper was added. I'll add a patch to delete them and
> clean up the return voids.
Yeah I think you got it - the functions had to be there pre-radix
because they were required by core code, and when radix was added
it probably just followed a template. Removing empty hash__ functions
should be fine I think.
Thanks,
Nick
next prev parent reply other threads:[~2022-11-07 12:29 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-25 4:44 [PATCH v9 0/7] Use per-CPU temporary mappings for patching on Radix MMU Benjamin Gray
2022-10-25 4:44 ` [PATCH v9 1/7] powerpc: Allow clearing and restoring registers independent of saved breakpoint state Benjamin Gray
2022-10-25 4:44 ` [PATCH v9 2/7] powerpc/code-patching: Handle RWX patching initialisation error Benjamin Gray
2022-11-02 9:36 ` Christophe Leroy
2022-11-02 22:37 ` Benjamin Gray
2022-10-25 4:44 ` [PATCH v9 3/7] powerpc/code-patching: Use WARN_ON and fix check in poking_init Benjamin Gray
2022-11-02 9:38 ` Christophe Leroy
2022-11-02 22:42 ` Benjamin Gray
2022-10-25 4:44 ` [PATCH v9 4/7] powerpc/code-patching: Verify instruction patch succeeded Benjamin Gray
2022-10-26 0:47 ` Benjamin Gray
2022-11-02 9:43 ` Christophe Leroy
2022-11-02 10:13 ` Christophe Leroy
2022-11-02 23:02 ` Benjamin Gray
2022-11-02 22:58 ` Benjamin Gray
2022-10-25 4:44 ` [PATCH v9 5/7] powerpc/tlb: Add local flush for page given mm_struct and psize Benjamin Gray
2022-11-02 9:56 ` Christophe Leroy
2022-11-03 0:39 ` Benjamin Gray
2022-11-03 0:45 ` Andrew Donnellan
2022-11-07 6:58 ` Benjamin Gray
2022-11-07 12:28 ` Nicholas Piggin [this message]
2022-10-25 4:44 ` [PATCH v9 6/7] powerpc/code-patching: Use temporary mm for Radix MMU Benjamin Gray
2022-11-02 10:11 ` Christophe Leroy
2022-11-03 3:10 ` Benjamin Gray
2022-11-08 5:16 ` Benjamin Gray
2022-10-25 4:44 ` [PATCH v9 7/7] powerpc/code-patching: Consolidate and cache per-cpu patching context Benjamin Gray
2022-11-02 10:17 ` Christophe Leroy
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=CO62AX0EBW9Y.2SCODZ31UQD4N@bobo \
--to=npiggin@gmail$(echo .)com \
--cc=ajd@linux$(echo .)ibm.com \
--cc=bgray@linux$(echo .)ibm.com \
--cc=christophe.leroy@csgroup$(echo .)eu \
--cc=cmr@bluescreens$(echo .)de \
--cc=jniethe5@gmail$(echo .)com \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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