Re: [PATCH v14 2/3] sched: Move task_mm_cid_work to mm timer

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

On 2025-07-07 10:48, Gabriele Monaco wrote:
[...]
>   
> -void task_tick_mm_cid(struct rq *rq, struct task_struct *curr)
> +void task_queue_mm_cid(struct task_struct *curr)
>   {
> -	struct callback_head *work = &curr->cid_work;
> -	unsigned long now = jiffies;
> +	int requeued;
>   
> -	if (!curr->mm || (curr->flags & (PF_EXITING | PF_KTHREAD)) ||
> -	    work->next != work)
> -		return;
> -	if (time_before(now, READ_ONCE(curr->mm->mm_cid_next_scan)))
> -		return;
> -
> -	/* No page allocation under rq lock */
> -	task_work_add(curr, work, TWA_RESUME);
> +	/*
> +	 * @curr must be a user thread and the timer must not be pending.
> +	 * Access to this timer is not serialised across threads sharing the
> +	 * same mm: ensure racing threads don't postpone enqueued timers and
> +	 * don't mmgrab() if they didn't enqueue the timer themselves.
> +	 * mmgrab() is necessary to ensure the mm exists until the timer runs.
> +	 */
> +	requeued = timer_reduce(&curr->mm->cid_timer,
> +				jiffies + msecs_to_jiffies(MM_CID_SCAN_DELAY));
> +	if (!requeued && timer_pending(&curr->mm->cid_timer))
> +		mmgrab(curr->mm);
>   }
>   
In v13 we had:

-	task_work_add(curr, work, TWA_RESUME);
+/* Call only when curr is a user thread. */
+void task_queue_mm_cid(struct task_struct *curr)
+{
+	/* Ensure the mm exists when we run. */
+	mmgrab(curr->mm);
+	queue_work(system_unbound_wq, &curr->mm->cid_work);
  }

The new pattern is to do mmgrab *after* timer_reduce has enqueued
the timer. This seems to be racy with timer execution. What prevents
the timer to run before mmgrab() is done ?

Thanks,

Mathieu

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