[PATCH 1/2] rv/nomiss: fix task storage lifetime

[PATCH 1/2] rv/nomiss: fix task storage lifetime

[Li Qiang]

The nomiss monitor stores a pointer to a task's embedded deadline entity
in per-PID object storage. The exit handler only removed that storage when
the task was still SCHED_DEADLINE. A task that changed to another policy
before exiting left its entry behind. Once the PID was reused, the new task
could reuse the old entry and dereference the freed deadline entity.

Remove storage unconditionally at sched_process_exit. Serialize creation
and destruction with locks sharded by PID, and do not create storage once
PF_EXITING is set. Each shard tracks both stored entries and in-flight
creators, allowing exits for shards without storage to avoid locking and
hash lookups while preserving the create/exit ordering.

Fixes: b133207deb72 ("rv: Add nomiss deadline monitor")
Signed-off-by: Li Qiang <liqiang01@kylinos.cn>
---
 kernel/trace/rv/monitors/deadline/deadline.h | 86 +++++++++++++++++++-
 kernel/trace/rv/monitors/nomiss/nomiss.c     |  2 +-
 2 files changed, 84 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/rv/monitors/deadline/deadline.h b/kernel/trace/rv/monitors/deadline/deadline.h
index 78fca873d61e..b92480e79807 100644
--- a/kernel/trace/rv/monitors/deadline/deadline.h
+++ b/kernel/trace/rv/monitors/deadline/deadline.h
@@ -1,6 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 
 #include <linux/kernel.h>
+#include <linux/hash.h>
+#include <linux/spinlock.h>
 #include <linux/uaccess.h>
 #include <linux/sched/deadline.h>
 #include <asm/syscall.h>
@@ -148,6 +150,37 @@ static inline struct sched_dl_entity *get_server(struct task_struct *tsk, u8 typ
 	return NULL;
 }
 
+#define DEADLINE_STORAGE_LOCK_BITS	6
+#define DEADLINE_STORAGE_LOCKS		BIT(DEADLINE_STORAGE_LOCK_BITS)
+
+/*
+ * A shard tracks both entries and creators which have not yet checked
+ * PF_EXITING. This lets unrelated exits avoid taking a storage lock while
+ * preserving the create/exit ordering.
+ */
+struct deadline_storage_shard {
+	raw_spinlock_t lock;
+	atomic_t users;
+} ____cacheline_aligned_in_smp;
+
+static struct deadline_storage_shard
+	deadline_storage_shards[DEADLINE_STORAGE_LOCKS];
+
+static inline struct deadline_storage_shard *deadline_storage_shard(int pid)
+{
+	return &deadline_storage_shards[hash_32(pid, DEADLINE_STORAGE_LOCK_BITS)];
+}
+
+static void deadline_storage_init(void)
+{
+	int i;
+
+	for (i = 0; i < DEADLINE_STORAGE_LOCKS; i++) {
+		raw_spin_lock_init(&deadline_storage_shards[i].lock);
+		atomic_set(&deadline_storage_shards[i].users, 0);
+	}
+}
+
 /*
  * Initialise monitors for all tasks and pre-allocate the storage for servers.
  * This is necessary since we don't have access to the servers here and
@@ -159,6 +192,8 @@ static inline int init_storage(bool skip_tasks)
 	struct task_struct *g, *p;
 	int cpu;
 
+	deadline_storage_init();
+
 	for_each_possible_cpu(cpu) {
 		if (!da_create_empty_storage(fair_server_id(cpu)))
 			goto fail;
@@ -177,6 +212,7 @@ static inline int init_storage(bool skip_tasks)
 				read_unlock(&tasklist_lock);
 				goto fail;
 			}
+			atomic_inc(&deadline_storage_shard(p->pid)->users);
 		}
 	}
 	read_unlock(&tasklist_lock);
@@ -187,17 +223,61 @@ static inline int init_storage(bool skip_tasks)
 	return -ENOMEM;
 }
 
+static void deadline_create_task_storage(struct task_struct *task)
+{
+	struct deadline_storage_shard *shard = deadline_storage_shard(task->pid);
+	bool created = false;
+
+	/*
+	 * The temporary reference makes an in-flight creator visible to exit.
+	 * If storage is created, it becomes the reference owned by that entry.
+	 */
+	atomic_inc(&shard->users);
+	/* Pair with exit before it tests users without taking the shard lock. */
+	smp_mb__after_atomic();
+
+	raw_spin_lock(&shard->lock);
+	if (!(READ_ONCE(task->flags) & PF_EXITING)) {
+		guard(rcu)();
+		if (!da_get_monitor(EXPAND_ID_TASK(task)) &&
+		    da_create_storage(EXPAND_ID_TASK(task), NULL))
+			created = true;
+	}
+	raw_spin_unlock(&shard->lock);
+
+	if (!created)
+		atomic_dec(&shard->users);
+}
+
+static void deadline_destroy_task_storage(struct task_struct *task)
+{
+	struct deadline_storage_shard *shard = deadline_storage_shard(task->pid);
+
+	/* Pair with the creator barrier before taking the no-storage fast path. */
+	smp_mb();
+	if (!atomic_read(&shard->users))
+		return;
+
+	raw_spin_lock(&shard->lock);
+	guard(rcu)();
+	if (da_get_monitor(task->pid, NULL)) {
+		/* A task may leave SCHED_DEADLINE before exiting. */
+		da_destroy_storage(task->pid);
+		atomic_dec(&shard->users);
+	}
+	raw_spin_unlock(&shard->lock);
+}
+
 static void __maybe_unused handle_newtask(void *data, struct task_struct *task, u64 flags)
 {
 	/* Might be superfluous as tasks are not started with this policy.. */
 	if (task->policy == SCHED_DEADLINE)
-		da_create_storage(EXPAND_ID_TASK(task), NULL);
+		deadline_create_task_storage(task);
 }
 
 static void __maybe_unused handle_exit(void *data, struct task_struct *p, bool group_dead)
 {
-	if (p->policy == SCHED_DEADLINE)
-		da_destroy_storage(get_entity_id(&p->dl, DL_TASK, DL_TASK));
+	deadline_destroy_task_storage(p);
 }
 
 #endif
diff --git a/kernel/trace/rv/monitors/nomiss/nomiss.c b/kernel/trace/rv/monitors/nomiss/nomiss.c
index 8ead8783c29f..2e5ee681505b 100644
--- a/kernel/trace/rv/monitors/nomiss/nomiss.c
+++ b/kernel/trace/rv/monitors/nomiss/nomiss.c
@@ -214,7 +214,7 @@ static void handle_sys_enter(void *data, struct pt_regs *regs, long id)
 	if (p->policy == SCHED_DEADLINE)
 		da_reset(EXPAND_ID_TASK(p));
 	else if (new_policy == SCHED_DEADLINE)
-		da_create_or_get(EXPAND_ID_TASK(p));
+		deadline_create_task_storage(p);
 }
 
 static void handle_sched_wakeup(void *data, struct task_struct *tsk)
-- 
2.50.1

[PATCH 2/2] rv/nomiss: close enable-time task exit race

[Li Qiang]

The initial task scan runs before the first patch registers the task exit
tracepoint. A task can begin exiting after it was selected by the scan but
before the exit handler is attached. sched_process_exit runs before
exit_notify() removes the task from the task list, so the scan could then
store a pointer to the exiting task's deadline entity after its cleanup
event had already been missed.

Initialize the shard state and attach the new-task, exit, and syscall
tracepoints before the initial scan. Make the scan use the same
PF_EXITING-aware creation helper as dynamic policy changes. On failure,
detach those tracepoints before destroying the monitor storage.

Fixes: b133207deb72 ("rv: Add nomiss deadline monitor")
Signed-off-by: Li Qiang <liqiang01@kylinos.cn>
---
 kernel/trace/rv/monitors/deadline/deadline.h | 27 ++++++++++-------
 kernel/trace/rv/monitors/nomiss/nomiss.c     | 18 +++++++----
 2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/kernel/trace/rv/monitors/deadline/deadline.h b/kernel/trace/rv/monitors/deadline/deadline.h
index b92480e79807..4626d6e2b218 100644
--- a/kernel/trace/rv/monitors/deadline/deadline.h
+++ b/kernel/trace/rv/monitors/deadline/deadline.h
@@ -181,6 +181,8 @@ static void deadline_storage_init(void)
 	}
 }
 
+static int deadline_create_task_storage(struct task_struct *task);
+
 /*
  * Initialise monitors for all tasks and pre-allocate the storage for servers.
  * This is necessary since we don't have access to the servers here and
@@ -190,9 +192,7 @@ static void deadline_storage_init(void)
 static inline int init_storage(bool skip_tasks)
 {
 	struct task_struct *g, *p;
-	int cpu;
-
-	deadline_storage_init();
+	int cpu, ret;
 
 	for_each_possible_cpu(cpu) {
 		if (!da_create_empty_storage(fair_server_id(cpu)))
@@ -208,25 +208,25 @@ static inline int init_storage(bool skip_tasks)
 	read_lock(&tasklist_lock);
 	for_each_process_thread(g, p) {
 		if (p->policy == SCHED_DEADLINE) {
-			if (!da_create_storage(EXPAND_ID_TASK(p), NULL)) {
+			ret = deadline_create_task_storage(p);
+			if (ret && ret != -ESRCH) {
 				read_unlock(&tasklist_lock);
-				goto fail;
+				return ret;
 			}
-			atomic_inc(&deadline_storage_shard(p->pid)->users);
 		}
 	}
 	read_unlock(&tasklist_lock);
 	return 0;
 
 fail:
-	da_monitor_destroy();
 	return -ENOMEM;
 }
 
-static void deadline_create_task_storage(struct task_struct *task)
+static int deadline_create_task_storage(struct task_struct *task)
 {
 	struct deadline_storage_shard *shard = deadline_storage_shard(task->pid);
 	bool created = false;
+	int ret = -ESRCH;
 
 	/*
 	 * The temporary reference makes an in-flight creator visible to exit.
@@ -239,14 +239,21 @@ static void deadline_create_task_storage(struct task_struct *task)
 	raw_spin_lock(&shard->lock);
 	if (!(READ_ONCE(task->flags) & PF_EXITING)) {
 		guard(rcu)();
-		if (!da_get_monitor(EXPAND_ID_TASK(task)) &&
-		    da_create_storage(EXPAND_ID_TASK(task), NULL))
+		if (da_get_monitor(EXPAND_ID_TASK(task))) {
+			ret = 0;
+		} else if (da_create_storage(EXPAND_ID_TASK(task), NULL)) {
 			created = true;
+			ret = 0;
+		} else {
+			ret = -ENOMEM;
+		}
 	}
 	raw_spin_unlock(&shard->lock);
 
 	if (!created)
 		atomic_dec(&shard->users);
+
+	return ret;
 }
 
 static void deadline_destroy_task_storage(struct task_struct *task)
diff --git a/kernel/trace/rv/monitors/nomiss/nomiss.c b/kernel/trace/rv/monitors/nomiss/nomiss.c
index 2e5ee681505b..843262779a41 100644
--- a/kernel/trace/rv/monitors/nomiss/nomiss.c
+++ b/kernel/trace/rv/monitors/nomiss/nomiss.c
@@ -231,18 +231,26 @@ static int enable_nomiss(void)
 	if (retval)
 		return retval;
 
+	deadline_storage_init();
+	rv_attach_trace_probe("nomiss", task_newtask, handle_newtask);
+	rv_attach_trace_probe("nomiss", sched_process_exit, handle_exit);
+	if (!should_skip_syscall_handle())
+		rv_attach_trace_probe("nomiss", sys_enter, handle_sys_enter);
+
 	retval = init_storage(false);
-	if (retval)
+	if (retval) {
+		rv_detach_trace_probe("nomiss", task_newtask, handle_newtask);
+		rv_detach_trace_probe("nomiss", sched_process_exit, handle_exit);
+		if (!should_skip_syscall_handle())
+			rv_detach_trace_probe("nomiss", sys_enter, handle_sys_enter);
+		ha_monitor_destroy();
 		return retval;
+	}
 	rv_attach_trace_probe("nomiss", sched_dl_replenish_tp, handle_dl_replenish);
 	rv_attach_trace_probe("nomiss", sched_dl_throttle_tp, handle_dl_throttle);
 	rv_attach_trace_probe("nomiss", sched_dl_server_stop_tp, handle_dl_server_stop);
 	rv_attach_trace_probe("nomiss", sched_switch, handle_sched_switch);
 	rv_attach_trace_probe("nomiss", sched_wakeup, handle_sched_wakeup);
-	if (!should_skip_syscall_handle())
-		rv_attach_trace_probe("nomiss", sys_enter, handle_sys_enter);
-	rv_attach_trace_probe("nomiss", task_newtask, handle_newtask);
-	rv_attach_trace_probe("nomiss", sched_process_exit, handle_exit);
 
 	return 0;
 }
-- 
2.50.1

Re: [PATCH 1/2] rv/nomiss: fix task storage lifetime

[Gabriele Monaco]

On Wed, 2026-07-15 at 14:00 +0800, Li Qiang wrote:
> The nomiss monitor stores a pointer to a task's embedded deadline entity
> in per-PID object storage. The exit handler only removed that storage when
> the task was still SCHED_DEADLINE. A task that changed to another policy
> before exiting left its entry behind. Once the PID was reused, the new task
> could reuse the old entry and dereference the freed deadline entity.

Yes, this is actually an issue, but wouldn't it be solved by just deleting the
storage as soon as a task changes its policy out of DEADLINE ?

We are currently only resetting the monitor (so timers won't fire), but deleting
would probably be more appropriate.

Then there's the issue we are tracking the syscall entry, so we don't know if
that is going to succeed, a more robust solution would be to add a tracepoint
inside the actual __sched_setscheduler() (but maintainers may not want that) or
tracking the context (old policy) inside sys_enter and commit the result in
sys_exit only if it succeeds.

> Remove storage unconditionally at sched_process_exit. Serialize creation
> and destruction with locks sharded by PID, and do not create storage once
> PF_EXITING is set. Each shard tracks both stored entries and in-flight
> creators, allowing exits for shards without storage to avoid locking and
> hash lookups while preserving the create/exit ordering.

I don't get why we need this complex machinery. What races are you trying to
solve with this locks? Concurrent syscalls and newtask/exit can this even
happen?

Thanks,
Gabriele

> 
> Fixes: b133207deb72 ("rv: Add nomiss deadline monitor")
> Signed-off-by: Li Qiang <liqiang01@kylinos.cn>
> ---
>  kernel/trace/rv/monitors/deadline/deadline.h | 86 +++++++++++++++++++-
>  kernel/trace/rv/monitors/nomiss/nomiss.c     |  2 +-
>  2 files changed, 84 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/trace/rv/monitors/deadline/deadline.h
> b/kernel/trace/rv/monitors/deadline/deadline.h
> index 78fca873d61e..b92480e79807 100644
> --- a/kernel/trace/rv/monitors/deadline/deadline.h
> +++ b/kernel/trace/rv/monitors/deadline/deadline.h
> @@ -1,6 +1,8 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
>  
>  #include <linux/kernel.h>
> +#include <linux/hash.h>
> +#include <linux/spinlock.h>
>  #include <linux/uaccess.h>
>  #include <linux/sched/deadline.h>
>  #include <asm/syscall.h>
> @@ -148,6 +150,37 @@ static inline struct sched_dl_entity *get_server(struct
> task_struct *tsk, u8 typ
>  	return NULL;
>  }
>  
> +#define DEADLINE_STORAGE_LOCK_BITS	6
> +#define DEADLINE_STORAGE_LOCKS		BIT(DEADLINE_STORAGE_LOCK_BITS)
> +
> +/*
> + * A shard tracks both entries and creators which have not yet checked
> + * PF_EXITING. This lets unrelated exits avoid taking a storage lock while
> + * preserving the create/exit ordering.
> + */
> +struct deadline_storage_shard {
> +	raw_spinlock_t lock;
> +	atomic_t users;
> +} ____cacheline_aligned_in_smp;
> +
> +static struct deadline_storage_shard
> +	deadline_storage_shards[DEADLINE_STORAGE_LOCKS];
> +
> +static inline struct deadline_storage_shard *deadline_storage_shard(int pid)
> +{
> +	return &deadline_storage_shards[hash_32(pid,
> DEADLINE_STORAGE_LOCK_BITS)];
> +}
> +
> +static void deadline_storage_init(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < DEADLINE_STORAGE_LOCKS; i++) {
> +		raw_spin_lock_init(&deadline_storage_shards[i].lock);
> +		atomic_set(&deadline_storage_shards[i].users, 0);
> +	}
> +}
> +
>  /*
>   * Initialise monitors for all tasks and pre-allocate the storage for
> servers.
>   * This is necessary since we don't have access to the servers here and
> @@ -159,6 +192,8 @@ static inline int init_storage(bool skip_tasks)
>  	struct task_struct *g, *p;
>  	int cpu;
>  
> +	deadline_storage_init();
> +
>  	for_each_possible_cpu(cpu) {
>  		if (!da_create_empty_storage(fair_server_id(cpu)))
>  			goto fail;
> @@ -177,6 +212,7 @@ static inline int init_storage(bool skip_tasks)
>  				read_unlock(&tasklist_lock);
>  				goto fail;
>  			}
> +			atomic_inc(&deadline_storage_shard(p->pid)->users);
>  		}
>  	}
>  	read_unlock(&tasklist_lock);
> @@ -187,17 +223,61 @@ static inline int init_storage(bool skip_tasks)
>  	return -ENOMEM;
>  }
>  
> +static void deadline_create_task_storage(struct task_struct *task)
> +{
> +	struct deadline_storage_shard *shard = deadline_storage_shard(task-
> >pid);
> +	bool created = false;
> +
> +	/*
> +	 * The temporary reference makes an in-flight creator visible to
> exit.
> +	 * If storage is created, it becomes the reference owned by that
> entry.
> +	 */
> +	atomic_inc(&shard->users);
> +	/* Pair with exit before it tests users without taking the shard
> lock. */
> +	smp_mb__after_atomic();
> +
> +	raw_spin_lock(&shard->lock);
> +	if (!(READ_ONCE(task->flags) & PF_EXITING)) {
> +		guard(rcu)();
> +		if (!da_get_monitor(EXPAND_ID_TASK(task)) &&
> +		    da_create_storage(EXPAND_ID_TASK(task), NULL))
> +			created = true;
> +	}
> +	raw_spin_unlock(&shard->lock);
> +
> +	if (!created)
> +		atomic_dec(&shard->users);
> +}
> +
> +static void deadline_destroy_task_storage(struct task_struct *task)
> +{
> +	struct deadline_storage_shard *shard = deadline_storage_shard(task-
> >pid);
> +
> +	/* Pair with the creator barrier before taking the no-storage fast
> path. */
> +	smp_mb();
> +	if (!atomic_read(&shard->users))
> +		return;
> +
> +	raw_spin_lock(&shard->lock);
> +	guard(rcu)();
> +	if (da_get_monitor(task->pid, NULL)) {
> +		/* A task may leave SCHED_DEADLINE before exiting. */
> +		da_destroy_storage(task->pid);
> +		atomic_dec(&shard->users);
> +	}
> +	raw_spin_unlock(&shard->lock);
> +}
> +
>  static void __maybe_unused handle_newtask(void *data, struct task_struct
> *task, u64 flags)
>  {
>  	/* Might be superfluous as tasks are not started with this policy..
> */
>  	if (task->policy == SCHED_DEADLINE)
> -		da_create_storage(EXPAND_ID_TASK(task), NULL);
> +		deadline_create_task_storage(task);
>  }
>  
>  static void __maybe_unused handle_exit(void *data, struct task_struct *p,
> bool group_dead)
>  {
> -	if (p->policy == SCHED_DEADLINE)
> -		da_destroy_storage(get_entity_id(&p->dl, DL_TASK, DL_TASK));
> +	deadline_destroy_task_storage(p);
>  }
>  
>  #endif
> diff --git a/kernel/trace/rv/monitors/nomiss/nomiss.c
> b/kernel/trace/rv/monitors/nomiss/nomiss.c
> index 8ead8783c29f..2e5ee681505b 100644
> --- a/kernel/trace/rv/monitors/nomiss/nomiss.c
> +++ b/kernel/trace/rv/monitors/nomiss/nomiss.c
> @@ -214,7 +214,7 @@ static void handle_sys_enter(void *data, struct pt_regs
> *regs, long id)
>  	if (p->policy == SCHED_DEADLINE)
>  		da_reset(EXPAND_ID_TASK(p));
>  	else if (new_policy == SCHED_DEADLINE)
> -		da_create_or_get(EXPAND_ID_TASK(p));
> +		deadline_create_task_storage(p);
>  }
>  
>  static void handle_sched_wakeup(void *data, struct task_struct *tsk)