linux-kernel - Re: [PATCH v3] sched/deadline: fix the hang in dl_task

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <13da5b3f-938f-4536-86e2-7073f781f26a@amperemail.onmicrosoft.com>
Date: Tue, 3 Sep 2024 09:48:47 +0800
From: Shijie Huang <shijie@...eremail.onmicrosoft.com>
To: Peter Zijlstra <peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>
Cc: Huang Shijie <shijie@...amperecomputing.com>, mingo@...hat.com,
 patches@...erecomputing.com, vincent.guittot@...aro.org,
 dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
 mgorman@...e.de, vschneid@...hat.com, vineeth@...byteword.org,
 bristot@...nel.org, linux-kernel@...r.kernel.org, hongyan.xia2@....com
Subject: Re: [PATCH v3] sched/deadline: fix the hang in dl_task_of


On 2024/9/2 19:14, Peter Zijlstra wrote:
> Subject: sched/deadline: Fix schedstats vs deadline servers
> From: Huang Shijie<shijie@...amperecomputing.com>
> Date: Thu, 29 Aug 2024 11:11:11 +0800
>
> From: Huang Shijie<shijie@...amperecomputing.com>
>
> In dl_server_start(), when schedstats is enabled, the following
> happens:
>
>    dl_server_start()
>      dl_se->dl_server = 1;
>      enqueue_dl_entity()
>        update_stats_enqueue_dl()
>          __schedstats_from_dl_se()
>            dl_task_of()
>              BUG_ON(dl_server(dl_se));
>
> Since only tasks have schedstats and internal entries do not, avoid
> trying to update stats in this case.
>
> Fixes: 63ba8422f876 ("sched/deadline: Introduce deadline servers")
> Signed-off-by: Huang Shijie<shijie@...amperecomputing.com>
> Signed-off-by: Peter Zijlstra (Intel)<peterz@...radead.org>
> Link:https://lkml.kernel.org/r/20240829031111.12142-1-shijie@os.amperecomputing.com
> ---
>   kernel/sched/deadline.c |   38 ++++++++++++++++----------------------
>   1 file changed, 16 insertions(+), 22 deletions(-)
>
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -1896,46 +1896,40 @@ static inline bool __dl_less(struct rb_n
>   	return dl_time_before(__node_2_dle(a)->deadline, __node_2_dle(b)->deadline);
>   }
>   
> -static inline struct sched_statistics *
> +static __always_inline struct sched_statistics *
>   __schedstats_from_dl_se(struct sched_dl_entity *dl_se)
>   {
> +	if (!schedstat_enabled())
> +		return NULL;
> +
> +	if (dl_server(dl_se))
> +		return NULL;
> +
>   	return &dl_task_of(dl_se)->stats;
>   }
>   
>   static inline void
>   update_stats_wait_start_dl(struct dl_rq *dl_rq, struct sched_dl_entity *dl_se)
>   {
> -	struct sched_statistics *stats;
> -
> -	if (!schedstat_enabled())
> -		return;
> -
> -	stats = __schedstats_from_dl_se(dl_se);
> -	__update_stats_wait_start(rq_of_dl_rq(dl_rq), dl_task_of(dl_se), stats);
> +	struct sched_statistics *stats = __schedstats_from_dl_se(dl_se);
> +	if (stats)
> +		__update_stats_wait_start(rq_of_dl_rq(dl_rq), dl_task_of(dl_se), stats);
>   }
>   
>   static inline void
>   update_stats_wait_end_dl(struct dl_rq *dl_rq, struct sched_dl_entity *dl_se)
>   {
> -	struct sched_statistics *stats;
> -
> -	if (!schedstat_enabled())
> -		return;
> -
> -	stats = __schedstats_from_dl_se(dl_se);
> -	__update_stats_wait_end(rq_of_dl_rq(dl_rq), dl_task_of(dl_se), stats);
> +	struct sched_statistics *stats = __schedstats_from_dl_se(dl_se);
> +	if (stats)
> +		__update_stats_wait_end(rq_of_dl_rq(dl_rq), dl_task_of(dl_se), stats);
>   }
>   
>   static inline void
>   update_stats_enqueue_sleeper_dl(struct dl_rq *dl_rq, struct sched_dl_entity *dl_se)
>   {
> -	struct sched_statistics *stats;
> -
> -	if (!schedstat_enabled())
> -		return;
> -
> -	stats = __schedstats_from_dl_se(dl_se);
> -	__update_stats_enqueue_sleeper(rq_of_dl_rq(dl_rq), dl_task_of(dl_se), stats);
> +	struct sched_statistics *stats = __schedstats_from_dl_se(dl_se);
> +	if (stats)
> +		__update_stats_enqueue_sleeper(rq_of_dl_rq(dl_rq), dl_task_of(dl_se), stats);
>   }
>   
>   static inline void

Thanks Peter,

I am okay with this one.


Thanks

Huang Shijie