Re: [patch V4 14/36] rseq: Cache CPU ID and MM CID values

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

On 2025-09-08 17:31, Thomas Gleixner wrote:
> In preparation for rewriting RSEQ exit to user space handling provide
> storage to cache the CPU ID and MM CID values which were written to user
> space. That prepares for a quick check, which avoids the update when
> nothing changed.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 
> ---
>   include/linux/rseq.h        |    7 +++++--
>   include/linux/rseq_types.h  |   21 +++++++++++++++++++++
>   include/trace/events/rseq.h |    4 ++--
>   kernel/rseq.c               |    4 ++++
>   4 files changed, 32 insertions(+), 4 deletions(-)
> 
> --- a/include/linux/rseq.h
> +++ b/include/linux/rseq.h
> @@ -57,6 +57,7 @@ static inline void rseq_virt_userspace_e
>   static inline void rseq_reset(struct task_struct *t)
>   {
>   	memset(&t->rseq, 0, sizeof(t->rseq));
> +	t->rseq.ids.cpu_cid = ~0ULL;
>   }
>   
>   static inline void rseq_execve(struct task_struct *t)
> @@ -70,10 +71,12 @@ static inline void rseq_execve(struct ta
>    */
>   static inline void rseq_fork(struct task_struct *t, unsigned long clone_flags)
>   {
> -	if (clone_flags & CLONE_VM)
> +	if (clone_flags & CLONE_VM) {
>   		rseq_reset(t);
> -	else
> +	} else {
>   		t->rseq = current->rseq;
> +		t->rseq.ids.cpu_cid = ~0ULL;
> +	}
>   }
>   
>   #else /* CONFIG_RSEQ */
> --- a/include/linux/rseq_types.h
> +++ b/include/linux/rseq_types.h
> @@ -31,17 +31,38 @@ struct rseq_event {
>   };
>   
>   /**
> + * struct rseq_ids - Cache for ids, which need to be updated

need -> needs

> + * @cpu_cid:	Compound of @cpu_id and @mm_cid to make the
> + *		compiler emit a single compare on 64-bit
> + * @cpu_id:	The CPU ID which was written last to user space
> + * @mm_cid:	The MM CID which was written last to user space
> + *
> + * @cpu_id and @mm_cid are updated when the data is written to user space.
> + */
> +struct rseq_ids {
> +	union {
> +		u64		cpu_cid;
> +		struct {
> +			u32	cpu_id;
> +			u32	mm_cid;
> +		};
> +	};
> +};
> +
> +/**
>    * struct rseq_data - Storage for all rseq related data
>    * @usrptr:	Pointer to the registered user space RSEQ memory
>    * @len:	Length of the RSEQ region
>    * @sig:	Signature of critial section abort IPs
>    * @event:	Storage for event management
> + * @ids:	Storage for cached CPU ID and MM CID

It's far from clear from the diff, but is there a missing space at the
beginning of the line above ?

Other than that:

Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>


>    */
>   struct rseq_data {
>   	struct rseq __user		*usrptr;
>   	u32				len;
>   	u32				sig;
>   	struct rseq_event		event;
> +	struct rseq_ids			ids;
>   };
>   
>   #else /* CONFIG_RSEQ */
> --- a/include/trace/events/rseq.h
> +++ b/include/trace/events/rseq.h
> @@ -21,9 +21,9 @@ TRACE_EVENT(rseq_update,
>   	),
>   
>   	TP_fast_assign(
> -		__entry->cpu_id = raw_smp_processor_id();
> +		__entry->cpu_id = t->rseq.ids.cpu_id;
>   		__entry->node_id = cpu_to_node(__entry->cpu_id);
> -		__entry->mm_cid = task_mm_cid(t);
> +		__entry->mm_cid = t->rseq.ids.mm_cid;
>   	),
>   
>   	TP_printk("cpu_id=%d node_id=%d mm_cid=%d", __entry->cpu_id,
> --- a/kernel/rseq.c
> +++ b/kernel/rseq.c
> @@ -184,6 +184,10 @@ static int rseq_update_cpu_node_id(struc
>   	rseq_unsafe_put_user(t, node_id, node_id, efault_end);
>   	rseq_unsafe_put_user(t, mm_cid, mm_cid, efault_end);
>   
> +	/* Cache the user space values */
> +	t->rseq.ids.cpu_id = cpu_id;
> +	t->rseq.ids.mm_cid = mm_cid;
> +
>   	/*
>   	 * Additional feature fields added after ORIG_RSEQ_SIZE
>   	 * need to be conditionally updated only if
> 


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com