From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933818AbcIAPqw (ORCPT ); Thu, 1 Sep 2016 11:46:52 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:47632 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932973AbcIAPqt (ORCPT ); Thu, 1 Sep 2016 11:46:49 -0400 X-IBM-Helo: d24dlp02.br.ibm.com X-IBM-MailFrom: bauerman@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org From: Thiago Jung Bauermann To: linuxppc-dev@lists.ozlabs.org Cc: Rui Teng , linux-kernel@vger.kernel.org, paulus@samba.org Subject: Re: [PATCH] powerpc: Clean up tm_abort duplication in hash_utils_64.c Date: Thu, 01 Sep 2016 12:46:40 -0300 User-Agent: KMail/4.14.3 (Linux/4.4.0-34-generic; KDE/4.14.13; x86_64; ; ) In-Reply-To: <1472183410-18522-1-git-send-email-rui.teng@linux.vnet.ibm.com> References: <1472183410-18522-1-git-send-email-rui.teng@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16090115-0032-0000-0000-0000027D94AE X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16090115-0033-0000-0000-00000EC9B337 Message-Id: <17335881.L5K12VaGEe@hactar> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-09-01_07:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=1 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1609010173 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am Freitag, 26 August 2016, 11:50:10 schrieb Rui Teng: > The same logic appears twice and should probably be pulled out into a > function. > > Suggested-by: Michael Ellerman > Signed-off-by: Rui Teng > --- > arch/powerpc/mm/hash_utils_64.c | 45 > +++++++++++++++++------------------------ 1 file changed, 19 > insertions(+), 26 deletions(-) > > diff --git a/arch/powerpc/mm/hash_utils_64.c > b/arch/powerpc/mm/hash_utils_64.c index 0821556..69ef702 100644 > --- a/arch/powerpc/mm/hash_utils_64.c > +++ b/arch/powerpc/mm/hash_utils_64.c > @@ -1460,6 +1460,23 @@ out_exit: > local_irq_restore(flags); > } > > +/* > + * 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. > + */ > +void local_tm_abort(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); > + } > +} > + Since local_tm_abort is only used in this file, it should be static. Also, since both places calling it are guarded by CONFIG_PPC_TRANSACTIONAL_MEM, wouldn't it be cleaner if the #ifdef was here instead and the #else block defined an empty static inline function? Then the call sites wouldn't need to be guarded. -- []'s Thiago Jung Bauermann IBM Linux Technology Center