All of lore.kernel.org
 help / color / mirror / Atom feed
From: guoren@kernel.org
To: jszhang@kernel.org
Cc: aou@eecs.berkeley.edu, linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org, palmer@dabbelt.com,
	paul.walmsley@sifive.com, surenb@google.com,
	chenhuacai@kernel.org, Guo Ren <guoren@kernel.org>
Subject: Re: [PATCH] riscv: mm: try VMA lock-based page fault handling first
Date: Wed, 24 May 2023 01:02:59 -0400	[thread overview]
Message-ID: <20230524050259.104358-1-guoren@kernel.org> (raw)
In-Reply-To: <20230523165942.2630-1-jszhang@kernel.org>

> Attempt VMA lock-based page fault handling first, and fall back to the
> existing mmap_lock-based handling if that fails.
> 
> A simple running the ebizzy benchmark on Lichee Pi 4A shows that
> PER_VMA_LOCK can improve the ebizzy benchmark by about 32.68%. In
Good improvement, I think VMA lock is worth to support in riscv.

Please give more details about ebizzy, Is it 
https://github.com/linux-test-project/ltp/blob/master/utils/benchmark/ebizzy-0.3/ebizzy.c
?

> theory, the more CPUs, the bigger improvement, but I don't have any
> HW platform which has more than 4 CPUs.
> 
> This is the riscv variant of "x86/mm: try VMA lock-based page fault
> handling first".
>

How about add Link tag here:
Link: https://lwn.net/Articles/906852/

> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> Any performance numbers are welcome! Especially the numbers on HW
> platforms with 8 or more CPUs.
> 
>  arch/riscv/Kconfig    |  1 +
>  arch/riscv/mm/fault.c | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 62e84fee2cfd..b958f67f9a12 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -42,6 +42,7 @@ config RISCV
>  	select ARCH_SUPPORTS_DEBUG_PAGEALLOC if MMU
>  	select ARCH_SUPPORTS_HUGETLBFS if MMU
>  	select ARCH_SUPPORTS_PAGE_TABLE_CHECK if MMU
> +	select ARCH_SUPPORTS_PER_VMA_LOCK if MMU
>  	select ARCH_USE_MEMTEST
>  	select ARCH_USE_QUEUED_RWLOCKS
>  	select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU
> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> index 8685f85a7474..eccdddf26f4b 100644
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -286,6 +286,36 @@ void handle_page_fault(struct pt_regs *regs)
>  		flags |= FAULT_FLAG_WRITE;
>  	else if (cause == EXC_INST_PAGE_FAULT)
>  		flags |= FAULT_FLAG_INSTRUCTION;
> +#ifdef CONFIG_PER_VMA_LOCK
> +	if (!(flags & FAULT_FLAG_USER))
> +		goto lock_mmap;
> +
> +	vma = lock_vma_under_rcu(mm, addr);
> +	if (!vma)
> +		goto lock_mmap;
> +
> +	if (unlikely(access_error(cause, vma))) {
> +		vma_end_read(vma);
> +		goto lock_mmap;
> +	}
> +
> +	fault = handle_mm_fault(vma, addr, flags | FAULT_FLAG_VMA_LOCK, regs);
> +	vma_end_read(vma);
> +
> +	if (!(fault & VM_FAULT_RETRY)) {
> +		count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
> +		goto done;
> +	}
> +	count_vm_vma_lock_event(VMA_LOCK_RETRY);
> +
> +	if (fault_signal_pending(fault, regs)) {
> +		if (!user_mode(regs))
> +			no_context(regs, addr);
> +		return;
> +	}
> +lock_mmap:
> +#endif /* CONFIG_PER_VMA_LOCK */
> +
>  retry:
>  	mmap_read_lock(mm);
>  	vma = find_vma(mm, addr);
> @@ -355,6 +385,9 @@ void handle_page_fault(struct pt_regs *regs)
>  
>  	mmap_read_unlock(mm);
>  
> +#ifdef CONFIG_PER_VMA_LOCK
> +done:
> +#endif
It's very close to cd7f176aea5f ("arm64/mm: try VMA lock-based page fault 
handling first"), and I didn't find any problem. So:

Reviewed-by: Guo Ren <guoren@kernel.org>

F.Y.I Huacai Chen, maybe he also would be interesting this new feature.


>  	if (unlikely(fault & VM_FAULT_ERROR)) {
>  		tsk->thread.bad_cause = cause;
>  		mm_fault_error(regs, addr, fault);
> -- 
> 2.40.1

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: guoren@kernel.org
To: jszhang@kernel.org
Cc: aou@eecs.berkeley.edu, linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org, palmer@dabbelt.com,
	paul.walmsley@sifive.com, surenb@google.com,
	chenhuacai@kernel.org, Guo Ren <guoren@kernel.org>
Subject: Re: [PATCH] riscv: mm: try VMA lock-based page fault handling first
Date: Wed, 24 May 2023 01:02:59 -0400	[thread overview]
Message-ID: <20230524050259.104358-1-guoren@kernel.org> (raw)
In-Reply-To: <20230523165942.2630-1-jszhang@kernel.org>

> Attempt VMA lock-based page fault handling first, and fall back to the
> existing mmap_lock-based handling if that fails.
> 
> A simple running the ebizzy benchmark on Lichee Pi 4A shows that
> PER_VMA_LOCK can improve the ebizzy benchmark by about 32.68%. In
Good improvement, I think VMA lock is worth to support in riscv.

Please give more details about ebizzy, Is it 
https://github.com/linux-test-project/ltp/blob/master/utils/benchmark/ebizzy-0.3/ebizzy.c
?

> theory, the more CPUs, the bigger improvement, but I don't have any
> HW platform which has more than 4 CPUs.
> 
> This is the riscv variant of "x86/mm: try VMA lock-based page fault
> handling first".
>

How about add Link tag here:
Link: https://lwn.net/Articles/906852/

> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> Any performance numbers are welcome! Especially the numbers on HW
> platforms with 8 or more CPUs.
> 
>  arch/riscv/Kconfig    |  1 +
>  arch/riscv/mm/fault.c | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 62e84fee2cfd..b958f67f9a12 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -42,6 +42,7 @@ config RISCV
>  	select ARCH_SUPPORTS_DEBUG_PAGEALLOC if MMU
>  	select ARCH_SUPPORTS_HUGETLBFS if MMU
>  	select ARCH_SUPPORTS_PAGE_TABLE_CHECK if MMU
> +	select ARCH_SUPPORTS_PER_VMA_LOCK if MMU
>  	select ARCH_USE_MEMTEST
>  	select ARCH_USE_QUEUED_RWLOCKS
>  	select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU
> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> index 8685f85a7474..eccdddf26f4b 100644
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -286,6 +286,36 @@ void handle_page_fault(struct pt_regs *regs)
>  		flags |= FAULT_FLAG_WRITE;
>  	else if (cause == EXC_INST_PAGE_FAULT)
>  		flags |= FAULT_FLAG_INSTRUCTION;
> +#ifdef CONFIG_PER_VMA_LOCK
> +	if (!(flags & FAULT_FLAG_USER))
> +		goto lock_mmap;
> +
> +	vma = lock_vma_under_rcu(mm, addr);
> +	if (!vma)
> +		goto lock_mmap;
> +
> +	if (unlikely(access_error(cause, vma))) {
> +		vma_end_read(vma);
> +		goto lock_mmap;
> +	}
> +
> +	fault = handle_mm_fault(vma, addr, flags | FAULT_FLAG_VMA_LOCK, regs);
> +	vma_end_read(vma);
> +
> +	if (!(fault & VM_FAULT_RETRY)) {
> +		count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
> +		goto done;
> +	}
> +	count_vm_vma_lock_event(VMA_LOCK_RETRY);
> +
> +	if (fault_signal_pending(fault, regs)) {
> +		if (!user_mode(regs))
> +			no_context(regs, addr);
> +		return;
> +	}
> +lock_mmap:
> +#endif /* CONFIG_PER_VMA_LOCK */
> +
>  retry:
>  	mmap_read_lock(mm);
>  	vma = find_vma(mm, addr);
> @@ -355,6 +385,9 @@ void handle_page_fault(struct pt_regs *regs)
>  
>  	mmap_read_unlock(mm);
>  
> +#ifdef CONFIG_PER_VMA_LOCK
> +done:
> +#endif
It's very close to cd7f176aea5f ("arm64/mm: try VMA lock-based page fault 
handling first"), and I didn't find any problem. So:

Reviewed-by: Guo Ren <guoren@kernel.org>

F.Y.I Huacai Chen, maybe he also would be interesting this new feature.


>  	if (unlikely(fault & VM_FAULT_ERROR)) {
>  		tsk->thread.bad_cause = cause;
>  		mm_fault_error(regs, addr, fault);
> -- 
> 2.40.1

  parent reply	other threads:[~2023-05-24  5:03 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-23 16:59 [PATCH] riscv: mm: try VMA lock-based page fault handling first Jisheng Zhang
2023-05-23 16:59 ` Jisheng Zhang
2023-05-23 17:11 ` Jisheng Zhang
2023-05-23 17:11   ` Jisheng Zhang
2023-05-24  0:06   ` Jisheng Zhang
2023-05-24  0:06     ` Jisheng Zhang
2023-05-24  5:02 ` guoren [this message]
2023-05-24  5:02   ` guoren
2023-05-24  5:28   ` Suren Baghdasaryan
2023-05-24  5:28     ` Suren Baghdasaryan
2023-05-24 13:20   ` Jisheng Zhang
2023-05-24 13:20     ` Jisheng Zhang
2023-06-20 19:27 ` Palmer Dabbelt
2023-06-20 19:27   ` Palmer Dabbelt
2023-06-20 19:40 ` patchwork-bot+linux-riscv
2023-06-20 19:40   ` patchwork-bot+linux-riscv
2023-06-21  6:15 ` Kefeng Wang
2023-06-21  6:15   ` Kefeng Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230524050259.104358-1-guoren@kernel.org \
    --to=guoren@kernel.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=chenhuacai@kernel.org \
    --cc=jszhang@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=surenb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.