From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752044AbcIBCws (ORCPT ); Thu, 1 Sep 2016 22:52:48 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:43550 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751299AbcIBCwr (ORCPT ); Thu, 1 Sep 2016 22:52:47 -0400 X-IBM-Helo: d03dlp03.boulder.ibm.com X-IBM-MailFrom: rui.teng@linux.vnet.ibm.com Subject: Re: [PATCH] powerpc: Clean up tm_abort duplication in hash_utils_64.c To: Thiago Jung Bauermann , linuxppc-dev@lists.ozlabs.org References: <1472183410-18522-1-git-send-email-rui.teng@linux.vnet.ibm.com> <17335881.L5K12VaGEe@hactar> Cc: linux-kernel@vger.kernel.org, paulus@samba.org From: Rui Teng Date: Fri, 2 Sep 2016 10:52:38 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <17335881.L5K12VaGEe@hactar> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16090202-8235-0000-0000-0000091BFE04 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00005695; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000184; SDB=6.00752727; UDB=6.00355943; IPR=6.00525185; BA=6.00004686; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00012555; XFM=3.00000011; UTC=2016-09-02 02:52:44 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16090202-8236-0000-0000-0000347AEDE3 Message-Id: <6642c870-763d-c896-aab4-a1bc36bb9015@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-09-01_10:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1609020039 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 9/1/16 11:46 PM, Thiago Jung Bauermann wrote: > 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. OK > > 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. I have considered this style before, but I am worried about the call stacks increased by empty function and forgot the inline function. Will send v2 with your comments. Thanks! >