[v3, 3/5] riscv: sched: defer restoring Vector context for user
Conor Dooley
conor at kernel.org
Tue Oct 24 04:24:20 PDT 2023
Hey Andy,
On Thu, Oct 19, 2023 at 03:45:50PM +0000, Andy Chiu wrote:
> User will use its Vector registers only after the kernel really returns
> to the userspace. So we can delay restoring Vector registers as long as
> we are still running in kernel mode. So, add a thread flag to indicates
> the need of restoring Vector and do the restore at the last
> arch-specific exit-to-user hook. This save the context restoring cost
> when we switch over multiple processes that run V in kernel mode. For
> example, if the kernel performs a context swicth from A->B->C, and
> returns to C's userspace, then there is no need to restore B's
> V-register.
>
> Besides, this also prevents us from repeatedly restoring V context when
> executing kernel-mode Vector multiple times for the upcoming kenel-mode
> Vector patches.
This comment now seems misplaced, as this patch has moved after adding
kernel mode vector in the series.
> The cost of this is that we must disable preemption and mark vector as
> busy during vstate_{save,restore}. Because then the V context will not
> get restored back immediately when a trap-causing context switch happens
> in the middle of vstate_{save,restore}.
>
> Signed-off-by: Andy Chiu <andy.chiu at sifive.com>
> ---
> Changelog v3:
> - Guard {get,put}_cpu_vector_context between vstate_* operation and
> explain it in the commit msg.
> - Drop R-b from Björn and A-b from Conor.
You can keep mine,
Acked-by: Conor Dooley <conor.dooley at microchip.com>
> Changelog v2:
> - rename and add comment for the new thread flag (Conor)
> ---
> arch/riscv/include/asm/entry-common.h | 17 +++++++++++++++++
> arch/riscv/include/asm/thread_info.h | 2 ++
> arch/riscv/include/asm/vector.h | 11 ++++++++++-
> arch/riscv/kernel/kernel_mode_vector.c | 2 +-
> arch/riscv/kernel/process.c | 2 ++
> arch/riscv/kernel/ptrace.c | 5 ++++-
> arch/riscv/kernel/signal.c | 5 ++++-
> arch/riscv/kernel/vector.c | 2 +-
> 8 files changed, 41 insertions(+), 5 deletions(-)
>
> diff --git a/arch/riscv/include/asm/entry-common.h b/arch/riscv/include/asm/entry-common.h
> index 6e4dee49d84b..8d64f1c18169 100644
> --- a/arch/riscv/include/asm/entry-common.h
> +++ b/arch/riscv/include/asm/entry-common.h
> @@ -4,6 +4,23 @@
> #define _ASM_RISCV_ENTRY_COMMON_H
>
> #include <asm/stacktrace.h>
> +#include <asm/thread_info.h>
> +#include <asm/vector.h>
> +
> +static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
> + unsigned long ti_work)
> +{
> + if (ti_work & _TIF_RISCV_V_DEFER_RESTORE) {
> + clear_thread_flag(TIF_RISCV_V_DEFER_RESTORE);
> + /*
> + * We are already called with irq disabled, so go without
> + * keepping track of vector_context_busy.
nit: s/keepping/keeping/
Cheers,
Conor.
> + */
> + riscv_v_vstate_restore(current, regs);
> + }
> +}
> +
> +#define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare
>
> void handle_page_fault(struct pt_regs *regs);
> void handle_break(struct pt_regs *regs);
> diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
> index 1833beb00489..b182f2d03e25 100644
> --- a/arch/riscv/include/asm/thread_info.h
> +++ b/arch/riscv/include/asm/thread_info.h
> @@ -93,12 +93,14 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
> #define TIF_NOTIFY_SIGNAL 9 /* signal notifications exist */
> #define TIF_UPROBE 10 /* uprobe breakpoint or singlestep */
> #define TIF_32BIT 11 /* compat-mode 32bit process */
> +#define TIF_RISCV_V_DEFER_RESTORE 12 /* restore Vector before returing to user */
>
> #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
> #define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
> #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)
> #define _TIF_NOTIFY_SIGNAL (1 << TIF_NOTIFY_SIGNAL)
> #define _TIF_UPROBE (1 << TIF_UPROBE)
> +#define _TIF_RISCV_V_DEFER_RESTORE (1 << TIF_RISCV_V_DEFER_RESTORE)
>
> #define _TIF_WORK_MASK \
> (_TIF_NOTIFY_RESUME | _TIF_SIGPENDING | _TIF_NEED_RESCHED | \
> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> index 8b8ece690ea1..2f11c6f3ad96 100644
> --- a/arch/riscv/include/asm/vector.h
> +++ b/arch/riscv/include/asm/vector.h
> @@ -188,6 +188,15 @@ static inline void riscv_v_vstate_restore(struct task_struct *task,
> }
> }
>
> +static inline void riscv_v_vstate_set_restore(struct task_struct *task,
> + struct pt_regs *regs)
> +{
> + if ((regs->status & SR_VS) != SR_VS_OFF) {
> + set_tsk_thread_flag(task, TIF_RISCV_V_DEFER_RESTORE);
> + riscv_v_vstate_on(regs);
> + }
> +}
> +
> static inline void __switch_to_vector(struct task_struct *prev,
> struct task_struct *next)
> {
> @@ -195,7 +204,7 @@ static inline void __switch_to_vector(struct task_struct *prev,
>
> regs = task_pt_regs(prev);
> riscv_v_vstate_save(prev, regs);
> - riscv_v_vstate_restore(next, task_pt_regs(next));
> + riscv_v_vstate_set_restore(next, task_pt_regs(next));
> }
>
> void riscv_v_vstate_ctrl_init(struct task_struct *tsk);
> diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
> index 74936e108771..fa01dc62256f 100644
> --- a/arch/riscv/kernel/kernel_mode_vector.c
> +++ b/arch/riscv/kernel/kernel_mode_vector.c
> @@ -90,7 +90,7 @@ void kernel_vector_end(void)
> if (WARN_ON(!has_vector()))
> return;
>
> - riscv_v_vstate_restore(current, task_pt_regs(current));
> + riscv_v_vstate_set_restore(current, task_pt_regs(current));
>
> riscv_v_disable();
>
> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
> index e32d737e039f..ec89e7edb6fd 100644
> --- a/arch/riscv/kernel/process.c
> +++ b/arch/riscv/kernel/process.c
> @@ -153,6 +153,7 @@ void flush_thread(void)
> riscv_v_vstate_off(task_pt_regs(current));
> kfree(current->thread.vstate.datap);
> memset(¤t->thread.vstate, 0, sizeof(struct __riscv_v_ext_state));
> + clear_tsk_thread_flag(current, TIF_RISCV_V_DEFER_RESTORE);
> #endif
> }
>
> @@ -169,6 +170,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
> *dst = *src;
> /* clear entire V context, including datap for a new task */
> memset(&dst->thread.vstate, 0, sizeof(struct __riscv_v_ext_state));
> + clear_tsk_thread_flag(dst, TIF_RISCV_V_DEFER_RESTORE);
>
> return 0;
> }
> diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
> index 2afe460de16a..7b93bcbdf9fa 100644
> --- a/arch/riscv/kernel/ptrace.c
> +++ b/arch/riscv/kernel/ptrace.c
> @@ -99,8 +99,11 @@ static int riscv_vr_get(struct task_struct *target,
> * Ensure the vector registers have been saved to the memory before
> * copying them to membuf.
> */
> - if (target == current)
> + if (target == current) {
> + get_cpu_vector_context();
> riscv_v_vstate_save(current, task_pt_regs(current));
> + put_cpu_vector_context();
> + }
>
> ptrace_vstate.vstart = vstate->vstart;
> ptrace_vstate.vl = vstate->vl;
> diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
> index 180d951d3624..d31d2c74d31f 100644
> --- a/arch/riscv/kernel/signal.c
> +++ b/arch/riscv/kernel/signal.c
> @@ -86,7 +86,10 @@ static long save_v_state(struct pt_regs *regs, void __user **sc_vec)
> /* datap is designed to be 16 byte aligned for better performance */
> WARN_ON(unlikely(!IS_ALIGNED((unsigned long)datap, 16)));
>
> + get_cpu_vector_context();
> riscv_v_vstate_save(current, regs);
> + put_cpu_vector_context();
> +
> /* Copy everything of vstate but datap. */
> err = __copy_to_user(&state->v_state, ¤t->thread.vstate,
> offsetof(struct __riscv_v_ext_state, datap));
> @@ -134,7 +137,7 @@ static long __restore_v_state(struct pt_regs *regs, void __user *sc_vec)
> if (unlikely(err))
> return err;
>
> - riscv_v_vstate_restore(current, regs);
> + riscv_v_vstate_set_restore(current, regs);
>
> return err;
> }
> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
> index 8d92fb6c522c..9d583b760db4 100644
> --- a/arch/riscv/kernel/vector.c
> +++ b/arch/riscv/kernel/vector.c
> @@ -167,7 +167,7 @@ bool riscv_v_first_use_handler(struct pt_regs *regs)
> return true;
> }
> riscv_v_vstate_on(regs);
> - riscv_v_vstate_restore(current, regs);
> + riscv_v_vstate_set_restore(current, regs);
> return true;
> }
>
> --
> 2.17.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-riscv/attachments/20231024/7d56e5eb/attachment.sig>
More information about the linux-riscv
mailing list