Re: [PATCH] powerpc/tm: Clean up duplication of code
From: Balbir Singh <bsingharora@gmail.com>
Date: 2016-05-11 05:03:26
On 11/05/16 14:55, Rashmica Gupta wrote:
quoted hunk ↗ jump to hunk
The same logic for tm_abort appears twice, so pull it out into a function. Signed-off-by: Rashmica Gupta <redacted> --- arch/powerpc/mm/hash_utils_64.c | 47 ++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 26 deletions(-)diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c index 7635b1c6b5da..1cef8f5aee9b 100644 --- a/arch/powerpc/mm/hash_utils_64.c +++ b/arch/powerpc/mm/hash_utils_64.c@@ -1318,6 +1318,25 @@ out_exit: local_irq_restore(flags); } +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM + /* Transactions are not aborted by tlbiel, only tlbie. + * Without, syncing a page back to a block device w/ PIO could pick up + * transactional data (bad!) so we force an abort here. Before the + * sync the page will be made read-only, which will flush_hash_page. + * BIG ISSUE here: if the kernel uses a page from userspace without + * unmapping it first, it may see the speculated version. + */ +static inline void abort_tm(int local) +{ + if (local && cpu_has_feature(CPU_FTR_TM) && + current->thread.regs && + MSR_TM_ACTIVE(current->thread.regs->msr)) { + tm_enable(); + tm_abort(TM_CAUSE_TLBI); + } +}
While your at this do
#else
static inline void abort_tm(int local)
{
}
quoted hunk ↗ jump to hunk
+#endif + /* WARNING: This is called from hash_low_64.S, if you change this prototype, * do not forget to update the assembly call site ! */@@ -1344,19 +1363,7 @@ void flush_hash_page(unsigned long vpn, real_pte_t pte, int psize, int ssize, } pte_iterate_hashed_end(); #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
Then remove these extra #ifdef
quoted hunk ↗ jump to hunk
- /* Transactions are not aborted by tlbiel, only tlbie. - * Without, syncing a page back to a block device w/ PIO could pick up - * transactional data (bad!) so we force an abort here. Before the - * sync the page will be made read-only, which will flush_hash_page. - * BIG ISSUE here: if the kernel uses a page from userspace without - * unmapping it first, it may see the speculated version. - */ - if (local && cpu_has_feature(CPU_FTR_TM) && - current->thread.regs && - MSR_TM_ACTIVE(current->thread.regs->msr)) { - tm_enable(); - tm_abort(TM_CAUSE_TLBI); - } + abort_tm(local); #endif }@@ -1415,19 +1422,7 @@ void flush_hash_hugepage(unsigned long vsid, unsigned long addr, } tm_abort: #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
Then remove these extra #ifdef
- /* Transactions are not aborted by tlbiel, only tlbie.
- * Without, syncing a page back to a block device w/ PIO could pick up
- * transactional data (bad!) so we force an abort here. Before the
- * sync the page will be made read-only, which will flush_hash_page.
- * BIG ISSUE here: if the kernel uses a page from userspace without
- * unmapping it first, it may see the speculated version.
- */
- if (local && cpu_has_feature(CPU_FTR_TM) &&
- current->thread.regs &&
- MSR_TM_ACTIVE(current->thread.regs->msr)) {
- tm_enable();
- tm_abort(TM_CAUSE_TLBI);
- }
+ abort_tm(local);
#endif
return;
}