Messages in this thread Patch in this message |  | | Subject | Re: [RFC PATCH 3/6] sched: Add over-utilization/tipping point indicator | From | Dietmar Eggemann <> | Date | Mon, 9 Apr 2018 11:53:00 +0200 |
| |
On 04/09/2018 11:40 AM, Peter Zijlstra wrote: > > (I know there is a new version out; but I was reading through this to > catch up with the discussion) > > On Tue, Mar 20, 2018 at 09:43:09AM +0000, Dietmar Eggemann wrote: >> +static inline int sd_overutilized(struct sched_domain *sd) >> +{ >> + return READ_ONCE(sd->shared->overutilized); >> +} >> + >> +static inline void update_overutilized_status(struct rq *rq) >> +{ >> + struct sched_domain *sd; >> + >> + rcu_read_lock(); >> + sd = rcu_dereference(rq->sd); >> + if (sd && !sd_overutilized(sd) && cpu_overutilized(rq->cpu)) >> + WRITE_ONCE(sd->shared->overutilized, 1); >> + rcu_read_unlock(); >> +} >> +#else > > I think you ought to go have a look at the end of > kernel/sched/topology.c:sd_init(), where it says: > > /* > * For all levels sharing cache; connect a sched_domain_shared > * instance. > */ > if (sd->flags & SD_SHARE_PKG_RESOURCES) { > sd->shared = *per_cpu_ptr(sdd->sds, sd_id); > atomic_inc(&sd->shared->ref); > atomic_set(&sd->shared->nr_busy_cpus, sd_weight); > } > > Because if I read all this correctly, your code assumes sd->shared > exists unconditionally, while the quoted bit only ensures it does so <= > LLC. >
But the patch changes this part further down.
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c index 64cc564f5255..c8b7c7665ab2 100644 --- a/kernel/sched/topology.c +++ b/kernel/sched/topology.c @@ -1184,15 +1184,11 @@ sd_init(struct sched_domain_topology_level *tl, sd->idle_idx = 1; }
- /* - * For all levels sharing cache; connect a sched_domain_shared - * instance. - */ - if (sd->flags & SD_SHARE_PKG_RESOURCES) { - sd->shared = *per_cpu_ptr(sdd->sds, sd_id); - atomic_inc(&sd->shared->ref); + sd->shared = *per_cpu_ptr(sdd->sds, sd_id); + atomic_inc(&sd->shared->ref); + + if (sd->flags & SD_SHARE_PKG_RESOURCES) atomic_set(&sd->shared->nr_busy_cpus, sd_weight); - }
|  |