Re: [PATCH 1/1] Sched/fair: Improve the accuracy of sched_stat_wait statistics
On Thu, Oct 15, 2020 at 02:48:46PM +0800, qianjun.ker...@gmail.com wrote: > From: jun qian > > When the sched_schedstat changes from 0 to 1, some sched se maybe > already in the runqueue, the se->statistics.wait_start will be 0. > So it will let the (rq_of(cfs_rq)) - se->statistics.wait_start) > wrong. We need to avoid this scenario. > > Signed-off-by: jun qian > Reviewed-by: Yafang Shao Thanks!
[PATCH 1/1] Sched/fair: Improve the accuracy of sched_stat_wait statistics
From: jun qian When the sched_schedstat changes from 0 to 1, some sched se maybe already in the runqueue, the se->statistics.wait_start will be 0. So it will let the (rq_of(cfs_rq)) - se->statistics.wait_start) wrong. We need to avoid this scenario. Signed-off-by: jun qian Reviewed-by: Yafang Shao --- kernel/sched/fair.c | 9 + 1 file changed, 9 insertions(+) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 1a68a05..6f8ca0c 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -906,6 +906,15 @@ static void update_curr_fair(struct rq *rq) if (!schedstat_enabled()) return; + /* +* When the sched_schedstat changes from 0 to 1, some sched se +* maybe already in the runqueue, the se->statistics.wait_start +* will be 0.So it will let the delta wrong. We need to avoid this +* scenario. +*/ + if (unlikely(!schedstat_val(se->statistics.wait_start))) + return; + delta = rq_clock(rq_of(cfs_rq)) - schedstat_val(se->statistics.wait_start); if (entity_is_task(se)) { -- 1.8.3.1
Re: [PATCH 1/1] Sched/fair: Improve the accuracy of sched_stat_wait statistics
On Wed, Oct 14, 2020 at 9:19 PM Peter Zijlstra wrote: > > On Fri, Oct 09, 2020 at 05:25:30PM +0800, qianjun.ker...@gmail.com wrote: > > From: jun qian > > > > When the sched_schedstat changes from 0 to 1, some sched se maybe > > already in the runqueue, the se->statistics.wait_start will be 0. > > So it will let the (rq_of(cfs_rq)) - se->statistics.wait_start) > > wrong. We need to avoid this scenario. > > > > Signed-off-by: jun qian > > Signed-off-by: Yafang Shao > > This SoB chain isn't valid. Did Yafang's tag need to a reviewed-by or > something? > This patch improves the behavior when sched_schedstat is changed from 0 to 1, so it looks good to me. Reviewed-by: Yafang Shao > > --- > > kernel/sched/fair.c | 9 + > > 1 file changed, 9 insertions(+) > > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > > index 1a68a05..6f8ca0c 100644 > > --- a/kernel/sched/fair.c > > +++ b/kernel/sched/fair.c > > @@ -906,6 +906,15 @@ static void update_curr_fair(struct rq *rq) > > if (!schedstat_enabled()) > > return; > > > > + /* > > + * When the sched_schedstat changes from 0 to 1, some sched se > > + * maybe already in the runqueue, the se->statistics.wait_start > > + * will be 0.So it will let the delta wrong. We need to avoid this > > + * scenario. > > + */ > > + if (unlikely(!schedstat_val(se->statistics.wait_start))) > > + return; > > + > > delta = rq_clock(rq_of(cfs_rq)) - > > schedstat_val(se->statistics.wait_start); > > > > if (entity_is_task(se)) { > > -- > > 1.8.3.1 > > -- Thanks Yafang
Re: [PATCH 1/1] Sched/fair: Improve the accuracy of sched_stat_wait statistics
On Fri, Oct 09, 2020 at 05:25:30PM +0800, qianjun.ker...@gmail.com wrote: > From: jun qian > > When the sched_schedstat changes from 0 to 1, some sched se maybe > already in the runqueue, the se->statistics.wait_start will be 0. > So it will let the (rq_of(cfs_rq)) - se->statistics.wait_start) > wrong. We need to avoid this scenario. > > Signed-off-by: jun qian > Signed-off-by: Yafang Shao This SoB chain isn't valid. Did Yafang's tag need to a reviewed-by or something? > --- > kernel/sched/fair.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index 1a68a05..6f8ca0c 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -906,6 +906,15 @@ static void update_curr_fair(struct rq *rq) > if (!schedstat_enabled()) > return; > > + /* > + * When the sched_schedstat changes from 0 to 1, some sched se > + * maybe already in the runqueue, the se->statistics.wait_start > + * will be 0.So it will let the delta wrong. We need to avoid this > + * scenario. > + */ > + if (unlikely(!schedstat_val(se->statistics.wait_start))) > + return; > + > delta = rq_clock(rq_of(cfs_rq)) - > schedstat_val(se->statistics.wait_start); > > if (entity_is_task(se)) { > -- > 1.8.3.1 >
[PATCH 1/1] Sched/fair: Improve the accuracy of sched_stat_wait statistics
From: jun qian When the sched_schedstat changes from 0 to 1, some sched se maybe already in the runqueue, the se->statistics.wait_start will be 0. So it will let the (rq_of(cfs_rq)) - se->statistics.wait_start) wrong. We need to avoid this scenario. Signed-off-by: jun qian Signed-off-by: Yafang Shao --- kernel/sched/fair.c | 9 + 1 file changed, 9 insertions(+) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 1a68a05..6f8ca0c 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -906,6 +906,15 @@ static void update_curr_fair(struct rq *rq) if (!schedstat_enabled()) return; + /* +* When the sched_schedstat changes from 0 to 1, some sched se +* maybe already in the runqueue, the se->statistics.wait_start +* will be 0.So it will let the delta wrong. We need to avoid this +* scenario. +*/ + if (unlikely(!schedstat_val(se->statistics.wait_start))) + return; + delta = rq_clock(rq_of(cfs_rq)) - schedstat_val(se->statistics.wait_start); if (entity_is_task(se)) { -- 1.8.3.1