lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 13 Feb 2024 16:48:50 +0000
From: Ryan Roberts <ryan.roberts@....com>
To: Mark Rutland <mark.rutland@....com>
Cc: Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Ard Biesheuvel <ardb@...nel.org>, Marc Zyngier <maz@...nel.org>,
 James Morse <james.morse@....com>, Andrey Ryabinin <ryabinin.a.a@...il.com>,
 Andrew Morton <akpm@...ux-foundation.org>,
 Matthew Wilcox <willy@...radead.org>, David Hildenbrand <david@...hat.com>,
 Kefeng Wang <wangkefeng.wang@...wei.com>, John Hubbard
 <jhubbard@...dia.com>, Zi Yan <ziy@...dia.com>,
 Barry Song <21cnbao@...il.com>, Alistair Popple <apopple@...dia.com>,
 Yang Shi <shy828301@...il.com>, Nicholas Piggin <npiggin@...il.com>,
 Christophe Leroy <christophe.leroy@...roup.eu>,
 "Aneesh Kumar K.V" <aneesh.kumar@...nel.org>,
 "Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>,
 Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 "H. Peter Anvin" <hpa@...or.com>, linux-arm-kernel@...ts.infradead.org,
 x86@...nel.org, linuxppc-dev@...ts.ozlabs.org, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 21/25] arm64/mm: Implement new
 [get_and_]clear_full_ptes() batch APIs

On 13/02/2024 16:43, Mark Rutland wrote:
> On Fri, Feb 02, 2024 at 08:07:52AM +0000, Ryan Roberts wrote:
>> Optimize the contpte implementation to fix some of the
>> exit/munmap/dontneed performance regression introduced by the initial
>> contpte commit. Subsequent patches will solve it entirely.
>>
>> During exit(), munmap() or madvise(MADV_DONTNEED), mappings must be
>> cleared. Previously this was done 1 PTE at a time. But the core-mm
>> supports batched clear via the new [get_and_]clear_full_ptes() APIs. So
>> let's implement those APIs and for fully covered contpte mappings, we no
>> longer need to unfold the contpte. This significantly reduces unfolding
>> operations, reducing the number of tlbis that must be issued.
>>
>> Tested-by: John Hubbard <jhubbard@...dia.com>
>> Signed-off-by: Ryan Roberts <ryan.roberts@....com>
>> ---
>>  arch/arm64/include/asm/pgtable.h | 67 ++++++++++++++++++++++++++++++++
>>  arch/arm64/mm/contpte.c          | 17 ++++++++
>>  2 files changed, 84 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index c07f0d563733..ad04adb7b87f 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -965,6 +965,37 @@ static inline pte_t __ptep_get_and_clear(struct mm_struct *mm,
>>  	return pte;
>>  }
>>  
>> +static inline void __clear_full_ptes(struct mm_struct *mm, unsigned long addr,
>> +				pte_t *ptep, unsigned int nr, int full)
>> +{
>> +	for (;;) {
>> +		__ptep_get_and_clear(mm, addr, ptep);
>> +		if (--nr == 0)
>> +			break;
>> +		ptep++;
>> +		addr += PAGE_SIZE;
>> +	}
>> +}
> 
> The loop construct is a bit odd; can't this be:

I found it a little odd at first, but its avoiding the ptep and addr increments
the last time through the loop. Its the preferred pattern for these functions in
core-mm. See default set_ptes(), wrprotect_ptes(), clear_full_ptes() in
include/linux/pgtable.h.

So I'd prefer to leave it as is so that we match them. What do you think?

> 
> 	while (nr--) {
> 		__ptep_get_and_clear(mm, addr, ptep);
> 		ptep++;
> 		addr += PAGE_SIZE;
> 	}
> 
> ... or:
> 
> 	do {
> 		__ptep_get_and_clear(mm, addr, ptep);
> 		ptep++;
> 		addr += PAGE_SIZE;
> 	} while (--nr);
> 
> ... ?
> 
> Otherwise, this looks good to me.
> 
> Mark.
> 
>> +
>> +static inline pte_t __get_and_clear_full_ptes(struct mm_struct *mm,
>> +				unsigned long addr, pte_t *ptep,
>> +				unsigned int nr, int full)
>> +{
>> +	pte_t pte, tmp_pte;
>> +
>> +	pte = __ptep_get_and_clear(mm, addr, ptep);
>> +	while (--nr) {
>> +		ptep++;
>> +		addr += PAGE_SIZE;
>> +		tmp_pte = __ptep_get_and_clear(mm, addr, ptep);
>> +		if (pte_dirty(tmp_pte))
>> +			pte = pte_mkdirty(pte);
>> +		if (pte_young(tmp_pte))
>> +			pte = pte_mkyoung(pte);
>> +	}
>> +	return pte;
>> +}
>> +
>>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>  #define __HAVE_ARCH_PMDP_HUGE_GET_AND_CLEAR
>>  static inline pmd_t pmdp_huge_get_and_clear(struct mm_struct *mm,
>> @@ -1167,6 +1198,11 @@ extern pte_t contpte_ptep_get(pte_t *ptep, pte_t orig_pte);
>>  extern pte_t contpte_ptep_get_lockless(pte_t *orig_ptep);
>>  extern void contpte_set_ptes(struct mm_struct *mm, unsigned long addr,
>>  				pte_t *ptep, pte_t pte, unsigned int nr);
>> +extern void contpte_clear_full_ptes(struct mm_struct *mm, unsigned long addr,
>> +				pte_t *ptep, unsigned int nr, int full);
>> +extern pte_t contpte_get_and_clear_full_ptes(struct mm_struct *mm,
>> +				unsigned long addr, pte_t *ptep,
>> +				unsigned int nr, int full);
>>  extern int contpte_ptep_test_and_clear_young(struct vm_area_struct *vma,
>>  				unsigned long addr, pte_t *ptep);
>>  extern int contpte_ptep_clear_flush_young(struct vm_area_struct *vma,
>> @@ -1254,6 +1290,35 @@ static inline void pte_clear(struct mm_struct *mm,
>>  	__pte_clear(mm, addr, ptep);
>>  }
>>  
>> +#define clear_full_ptes clear_full_ptes
>> +static inline void clear_full_ptes(struct mm_struct *mm, unsigned long addr,
>> +				pte_t *ptep, unsigned int nr, int full)
>> +{
>> +	if (likely(nr == 1)) {
>> +		contpte_try_unfold(mm, addr, ptep, __ptep_get(ptep));
>> +		__clear_full_ptes(mm, addr, ptep, nr, full);
>> +	} else {
>> +		contpte_clear_full_ptes(mm, addr, ptep, nr, full);
>> +	}
>> +}
>> +
>> +#define get_and_clear_full_ptes get_and_clear_full_ptes
>> +static inline pte_t get_and_clear_full_ptes(struct mm_struct *mm,
>> +				unsigned long addr, pte_t *ptep,
>> +				unsigned int nr, int full)
>> +{
>> +	pte_t pte;
>> +
>> +	if (likely(nr == 1)) {
>> +		contpte_try_unfold(mm, addr, ptep, __ptep_get(ptep));
>> +		pte = __get_and_clear_full_ptes(mm, addr, ptep, nr, full);
>> +	} else {
>> +		pte = contpte_get_and_clear_full_ptes(mm, addr, ptep, nr, full);
>> +	}
>> +
>> +	return pte;
>> +}
>> +
>>  #define __HAVE_ARCH_PTEP_GET_AND_CLEAR
>>  static inline pte_t ptep_get_and_clear(struct mm_struct *mm,
>>  				unsigned long addr, pte_t *ptep)
>> @@ -1338,6 +1403,8 @@ static inline int ptep_set_access_flags(struct vm_area_struct *vma,
>>  #define set_pte					__set_pte
>>  #define set_ptes				__set_ptes
>>  #define pte_clear				__pte_clear
>> +#define clear_full_ptes				__clear_full_ptes
>> +#define get_and_clear_full_ptes			__get_and_clear_full_ptes
>>  #define __HAVE_ARCH_PTEP_GET_AND_CLEAR
>>  #define ptep_get_and_clear			__ptep_get_and_clear
>>  #define __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG
>> diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
>> index c85e64baf03b..80346108450b 100644
>> --- a/arch/arm64/mm/contpte.c
>> +++ b/arch/arm64/mm/contpte.c
>> @@ -207,6 +207,23 @@ void contpte_set_ptes(struct mm_struct *mm, unsigned long addr,
>>  }
>>  EXPORT_SYMBOL(contpte_set_ptes);
>>  
>> +void contpte_clear_full_ptes(struct mm_struct *mm, unsigned long addr,
>> +				pte_t *ptep, unsigned int nr, int full)
>> +{
>> +	contpte_try_unfold_partial(mm, addr, ptep, nr);
>> +	__clear_full_ptes(mm, addr, ptep, nr, full);
>> +}
>> +EXPORT_SYMBOL(contpte_clear_full_ptes);
>> +
>> +pte_t contpte_get_and_clear_full_ptes(struct mm_struct *mm,
>> +				unsigned long addr, pte_t *ptep,
>> +				unsigned int nr, int full)
>> +{
>> +	contpte_try_unfold_partial(mm, addr, ptep, nr);
>> +	return __get_and_clear_full_ptes(mm, addr, ptep, nr, full);
>> +}
>> +EXPORT_SYMBOL(contpte_get_and_clear_full_ptes);
>> +
>>  int contpte_ptep_test_and_clear_young(struct vm_area_struct *vma,
>>  					unsigned long addr, pte_t *ptep)
>>  {
>> -- 
>> 2.25.1
>>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ