Re: [PATCH v2 07/10] rcu/tasks: Check RCU watching state for holdout idle injection tasks

[Frederic Weisbecker]

Le Wed, Oct 09, 2024 at 06:21:24PM +0530, neeraj.upadhyay@xxxxxxxxxx a écrit :
> From: Neeraj Upadhyay <neeraj.upadhyay@xxxxxxxxxx>
> 
> Use RCU watching state of a CPU to check whether RCU-tasks GP
> need to wait for idle injection task on that CPU. Idle injection
> tasks which are in deep-idle states where RCU is not watching or
> which have transitioned to/from deep-idle state do not block
> RCU-tasks grace period.
> 
> Signed-off-by: Neeraj Upadhyay <neeraj.upadhyay@xxxxxxxxxx>

For now this should work because there is a single user that is
a per-cpu kthread, therefore no RCU-watching writer can race
against another (real idle VS idle injection or idle_injection VS
idle injection) without going first through a voluntary context
switch. But who knows about the future? If an idle injection kthread
is preempted by another idle injection right after clearing PF_IDLE,
there could be some spurious QS accounted for the preempted
kthread.

So perhaps we can consider idle injection as any normal task and
wait for it to voluntary schedule?

Well I see DEFAULT_DURATION_JIFFIES = 6, which is 60 ms on HZ=100.

Yeah that's a lot...so perhaps this patch is needed after all...

> ---
>  kernel/rcu/tasks.h | 63 +++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 48 insertions(+), 15 deletions(-)
> 
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index d8506d2e6f54..1947f9b6346d 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -38,6 +38,8 @@ typedef void (*postgp_func_t)(struct rcu_tasks *rtp);
>   * @rtpp: Pointer to the rcu_tasks structure.
>   * @rcu_watching_snap: Per-GP RCU-watching snapshot for idle tasks.
>   * @rcu_watching_snap_rec: RCU-watching snapshot recorded for idle task.
> + * @rcu_watching_idle_inj_snap: Per-GP RCU-watching snapshot for idle inject task.
> + * @rcu_watching_idle_inj_rec: RCU-watching snapshot recorded for idle inject task.
>   */
>  struct rcu_tasks_percpu {
>  	struct rcu_segcblist cblist;
> @@ -56,6 +58,8 @@ struct rcu_tasks_percpu {
>  	struct rcu_tasks *rtpp;
>  	int rcu_watching_snap;
>  	bool rcu_watching_snap_rec;
> +	int rcu_watching_idle_inj_snap;
> +	bool rcu_watching_idle_inj_rec;

So how about:

struct rcu_watching_task {
    int snap;
    bool rec;
}
...
struct rcu_tasks_percpu {
       ...
       struct rcu_watching_task idle_task;
       struct rcu_watching_task idle_inject;
}

Thanks.