Messages in this thread | | | From | Vincent Guittot <> | Date | Mon, 14 Nov 2022 17:46:05 +0100 | Subject | Re: [PATCH v8 1/9] sched/fair: fix unfairness at wakeup |
| |
On Mon, 14 Nov 2022 at 17:20, Patrick Bellasi <patrick.bellasi@matbug.net> wrote: > > Hi Vincent! > > On 10-Nov 18:50, Vincent Guittot wrote: > > [...] > > > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h > > index 1fc198be1ffd..14879d429919 100644 > > --- a/kernel/sched/sched.h > > +++ b/kernel/sched/sched.h > > @@ -2432,9 +2432,9 @@ extern void check_preempt_curr(struct rq *rq, struct task_struct *p, int flags); > > extern const_debug unsigned int sysctl_sched_nr_migrate; > > extern const_debug unsigned int sysctl_sched_migration_cost; > > > > -#ifdef CONFIG_SCHED_DEBUG > > extern unsigned int sysctl_sched_latency; > > extern unsigned int sysctl_sched_min_granularity; > > +#ifdef CONFIG_SCHED_DEBUG > > extern unsigned int sysctl_sched_idle_min_granularity; > > extern unsigned int sysctl_sched_wakeup_granularity; > > extern int sysctl_resched_latency_warn_ms; > > @@ -2448,6 +2448,34 @@ extern unsigned int sysctl_numa_balancing_scan_period_max; > > extern unsigned int sysctl_numa_balancing_scan_size; > > #endif > > > > +static inline unsigned long get_sched_latency(bool idle) > ^^^^^^^^^^^^^^^^^ > > This can be confusing since it's not always returning the sysctl_sched_latency > value. It's also being used to tune the vruntime at wakeup time. > > Thus, what about renaming this to something more close to what's used for, e.g. > get_wakeup_latency(se) > ? > > Also, in the following patches we call this always with a false parametr. > Thus, perhaps in a following patch, we can better add something like: > #define max_wakeup_latency get_wakeup_latency(false) > ?
I'm going to rename get_wakeup_latency by get_sleep_latency() as proposed earlier.
I don't see the benefit of adding a macro of top so will keep the parameter
> > > +{ > > + unsigned long thresh; > > + > > + if (idle) > > + thresh = sysctl_sched_min_granularity; > > + else > > + thresh = sysctl_sched_latency; > > + > > + /* > > + * Halve their sleep time's effect, to allow > > + * for a gentler effect of sleepers: > > + */ > > + if (sched_feat(GENTLE_FAIR_SLEEPERS)) > > + thresh >>= 1; > > + > > + return thresh; > > +} > > + > > +static inline unsigned long get_latency_max(void) > ^^^^^^^^^^^^^^^ > > This is always used to cap some form of vruntime deltas in: > - check_preempt_tick() > - wakeup_latency_gran() > - wakeup_preempt_entity() > It's always smaller then the max_wakeup_latency (as defined above). > > Thus, does not seems something like: > wakeup_latency_threshold() > a better documenting naming? > > > +{ > > + unsigned long thresh = get_sched_latency(false); > > + > > + thresh -= sysctl_sched_min_granularity; > > + > > + return thresh; > > +} > > [...] > > Best, > Patrick > > -- > #include <best/regards.h> > > Patrick Bellasi >
| |