From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 90C0AC43331 for ; Thu, 26 Mar 2020 07:12:14 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5C21620772 for ; Thu, 26 Mar 2020 07:12:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5C21620772 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=huawei.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id DB53B6B000A; Thu, 26 Mar 2020 03:12:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D67F66B000C; Thu, 26 Mar 2020 03:12:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C7B996B000D; Thu, 26 Mar 2020 03:12:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0054.hostedemail.com [216.40.44.54]) by kanga.kvack.org (Postfix) with ESMTP id B155E6B000A for ; Thu, 26 Mar 2020 03:12:13 -0400 (EDT) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id BBE72585F for ; Thu, 26 Mar 2020 07:12:13 +0000 (UTC) X-FDA: 76636644546.03.band84_6a375d43cba35 X-HE-Tag: band84_6a375d43cba35 X-Filterd-Recvd-Size: 6212 Received: from huawei.com (szxga07-in.huawei.com [45.249.212.35]) by imf20.hostedemail.com (Postfix) with ESMTP for ; Thu, 26 Mar 2020 07:12:09 +0000 (UTC) Received: from DGGEMS403-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id 5CE04B5CB84A1637917B; Thu, 26 Mar 2020 15:11:34 +0800 (CST) Received: from [127.0.0.1] (10.173.220.25) by DGGEMS403-HUB.china.huawei.com (10.3.19.203) with Microsoft SMTP Server id 14.3.487.0; Thu, 26 Mar 2020 15:11:25 +0800 Subject: Re: [RFC PATCH v4 5/6] arm64: tlb: Use translation level hint in vm_flags To: Marc Zyngier CC: , , , , , , , , , , , , , , , , , , , , , , , References: <20200324134534.1570-1-yezhenyu2@huawei.com> <20200324134534.1570-6-yezhenyu2@huawei.com> <20200324144514.340c78d9@why> <986be927-02c6-3cc2-ca39-30ccad60eae0@huawei.com> <2f4cb3ef52c5589b388225e487651a2b@kernel.org> From: Zhenyu Ye Message-ID: <6e2fece0-a7d7-a470-6911-5891fd09a140@huawei.com> Date: Thu, 26 Mar 2020 15:11:23 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.3.0 MIME-Version: 1.0 In-Reply-To: <2f4cb3ef52c5589b388225e487651a2b@kernel.org> Content-Type: text/plain; charset="UTF-8" X-Originating-IP: [10.173.220.25] X-CFilter-Loop: Reflected Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Hi Marc, On 2020/3/25 22:13, Marc Zyngier wrote: >>>> >>>> +inline unsigned int get_vma_level(struct vm_area_struct *vma) >>>> +{ >>>> +=C2=A0=C2=A0=C2=A0 unsigned int level =3D 0; >>>> +=C2=A0=C2=A0=C2=A0 if (vma->vm_flags & VM_LEVEL_PUD) >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 level =3D 1; >>>> +=C2=A0=C2=A0=C2=A0 else if (vma->vm_flags & VM_LEVEL_PMD) >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 level =3D 2; >>>> +=C2=A0=C2=A0=C2=A0 else if (vma->vm_flags & VM_LEVEL_PTE) >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 level =3D 3; >>>> + >>>> +=C2=A0=C2=A0=C2=A0 vma->vm_flags &=3D ~(VM_LEVEL_PUD | VM_LEVEL_PMD= | VM_LEVEL_PTE); >>>> +=C2=A0=C2=A0=C2=A0 return level; >>>> +} >>>> + >>>> =C2=A0void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd) >>>> =C2=A0{ >>>> =C2=A0=C2=A0=C2=A0=C2=A0 pgd_t *fixmap_pgdp; >>> >>> >>> If feels bizarre a TLBI is now a destructive operation: you've lost t= he >>> flags by having cleared them. Even if that's not really a problem in >>> practice (you issue TLBI because you've unmapped the VMA), it remains >>> that the act of invalidating TLBs isn't expected to change a kernel >>> structure (and I'm not even thinking about potential races here). >> >> I cleared vm_flags here just out of caution, because every TLBI flush >> action should set vm_flags themself. As I know, the TLB_LEVEL of an vm= a >> will only be changed by transparent hugepage collapse and merge when >> the page is not mapped, so there may not have potential races. >> >> But you are right that TLBI should't change a kernel structure. >> I will remove the clear action and add some notices here. >=20 > More than that. You are changing the VMA flags at TLBI time already, > when calling get_vma_level(). That is already unacceptable -- I don't > think (and Peter will certainly correct me if I'm wrong) that you > are allowed to change the flags on that code path, as you probably > don't hold the write_lock. > Thanks for your review. I will avoid this problem in next version. >>> If anything, I feel this should be based around the mmu_gather >>> structure, which already tracks the right level of information and >>> additionally knows about the P4D level which is missing in your patch= es >>> (even if arm64 is so far limited to 4 levels). >>> >>> Thanks, >>> >>> =C2=A0=C2=A0=C2=A0=C2=A0M. >>> >> >> mmu_gather structure has tracked the level information, but we can onl= y >> use the info in the tlb_flush interface. If we want to use the info in >> flush_tlb_range, we may should have to add a parameter to this interfa= ce, >> such as: >> >> =C2=A0=C2=A0=C2=A0=C2=A0flush_tlb_range(vma, start, end, flags); >> >> However, the flush_tlb_range is a common interface to all architecture= s, >> I'm not sure if this is feasible because this feature is only supporte= d >> by ARM64 currently. >=20 > You could always build an on-stack mmu_gather structure and pass it dow= n > to the TLB invalidation helpers. >=20 > And frankly, you are not going to be able to fit such a change in the > way Linux deals with TLBs by adding hacks at the periphery. You'll need > to change some of the core abstractions. >=20 > Finally, as Peter mentioned separately, there is Power9 which has simil= ar > instructions, and could make use of it too. So that's yet another reaso= n > to stop hacking at the arch level. >=20 OK, I will try to add struct mmu_gather to flush_tlb_range, such as: void flush_tlb_range(struct mmu_gather *tlb, struct vm_area_struct *vma, unsigned long start, unsigned long end); This will involve all architectures, I will do it carefully. >> >> Or can we ignore the flush_tlb_range and only set the TTL field in >> tlb_flush?=C2=A0 Waiting for your suggestion... >=20 > You could, but you could also ignore TTL altogether (what's the point > in only having half of it?). See my suggestion above. >=20 >> For P4D level, the TTL field is limited to 4 bit(2 for translation gra= nule >> and 2 for page table level), so we can no longer afford more levels :)= . >=20 > You clearly didn't read the bit that said "even if arm64 is so far limi= ted > to 4 levels". But again, this is Linux, a multi-architecture kernel, an= d > not an arm64-special. Changes you make have to work for all architectur= es, > and be extensible enough for future changes. >=20 Using the struct mmu_gather to pass the TTL value will not have this problem :). Thanks, Zhenyu