On Saturday, September 30, 2017 9:20:28 AM CEST Aubrey Li wrote: > Record the overhead of idle entry in micro-second >
What is this needed for? > Signed-off-by: Aubrey Li <[email protected]> > --- > drivers/cpuidle/cpuidle.c | 33 +++++++++++++++++++++++++++++++++ > include/linux/cpuidle.h | 14 ++++++++++++++ > kernel/sched/idle.c | 8 +++++++- > 3 files changed, 54 insertions(+), 1 deletion(-) > > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c > index 60bb64f..4066308 100644 > --- a/drivers/cpuidle/cpuidle.c > +++ b/drivers/cpuidle/cpuidle.c > @@ -302,6 +302,39 @@ void cpuidle_reflect(struct cpuidle_device *dev, int > index) > cpuidle_curr_governor->reflect(dev, index); > } > > +/* cpuidle_entry_start - record idle entry start */ A proper kerneldoc, please. > +void cpuidle_entry_start(void) > +{ > + struct cpuidle_device *dev = cpuidle_get_device(); > + > + if (dev) > + dev->idle_stat.entry_start = local_clock(); > +} > + > +/* > + * cpuidle_entry_end - record idle entry end, and maintain > + * the entry overhead average in micro-second > + */ A proper kerneldoc, please. > +void cpuidle_entry_end(void) > +{ > + struct cpuidle_device *dev = cpuidle_get_device(); > + u64 overhead; > + s64 diff; > + > + if (dev) { > + dev->idle_stat.entry_end = local_clock(); > + overhead = div_u64(dev->idle_stat.entry_end - > + dev->idle_stat.entry_start, NSEC_PER_USEC); Is the conversion really necessary? If so, then why? And if there is a good reason, what about using right shift to do an approximate conversion to avoid the extra division here? > + diff = overhead - dev->idle_stat.overhead; > + dev->idle_stat.overhead += diff >> 3; Can you please explain what happens in the two lines above? > + /* > + * limit overhead to 1us > + */ > + if (dev->idle_stat.overhead == 0) > + dev->idle_stat.overhead = 1; > + } > +} > + > /** > * cpuidle_install_idle_handler - installs the cpuidle idle loop handler > */ > diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h > index fc1e5d7..cad9b71 100644 > --- a/include/linux/cpuidle.h > +++ b/include/linux/cpuidle.h > @@ -72,6 +72,15 @@ struct cpuidle_device_kobj; > struct cpuidle_state_kobj; > struct cpuidle_driver_kobj; > > +struct cpuidle_stat { > + u64 entry_start; /* nanosecond */ > + u64 entry_end; /* nanosecond */ > + u64 overhead; /* nanosecond */ > + unsigned int predicted_us; /* microsecond */ > + bool predicted; /* ever predicted? */ > + bool fast_idle; /* fast idle? */ Some of the fields here are never used in the code after this patch. Also it would be good to add a comment describing the meaning of the fields. > +}; > + > struct cpuidle_device { > unsigned int registered:1; > unsigned int enabled:1; > @@ -89,6 +98,7 @@ struct cpuidle_device { > cpumask_t coupled_cpus; > struct cpuidle_coupled *coupled; > #endif > + struct cpuidle_stat idle_stat; > }; > > DECLARE_PER_CPU(struct cpuidle_device *, cpuidle_devices); > @@ -131,6 +141,8 @@ extern bool cpuidle_not_available(struct cpuidle_driver > *drv, > > extern int cpuidle_select(struct cpuidle_driver *drv, > struct cpuidle_device *dev); > +extern void cpuidle_entry_start(void); > +extern void cpuidle_entry_end(void); > extern int cpuidle_enter(struct cpuidle_driver *drv, > struct cpuidle_device *dev, int index); > extern void cpuidle_reflect(struct cpuidle_device *dev, int index); > @@ -164,6 +176,8 @@ static inline bool cpuidle_not_available(struct > cpuidle_driver *drv, > static inline int cpuidle_select(struct cpuidle_driver *drv, > struct cpuidle_device *dev) > {return -ENODEV; } > +static inline void cpuidle_entry_start(void) { } > +static inline void cpuidle_entry_end(void) { } > static inline int cpuidle_enter(struct cpuidle_driver *drv, > struct cpuidle_device *dev, int index) > {return -ENODEV; } > diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c > index 6c23e30..0951dac 100644 > --- a/kernel/sched/idle.c > +++ b/kernel/sched/idle.c > @@ -210,6 +210,12 @@ static void cpuidle_idle_call(void) > static void do_idle(void) > { > /* > + * we record idle entry overhead now, so any deferrable items > + * in idle entry path need to be placed between cpuidle_entry_start() > + * and cpuidle_entry_end() > + */ > + cpuidle_entry_start(); > + /* > * If the arch has a polling bit, we maintain an invariant: > * > * Our polling bit is clear if we're not scheduled (i.e. if rq->curr != > @@ -217,10 +223,10 @@ static void do_idle(void) > * then setting need_resched is guaranteed to cause the CPU to > * reschedule. > */ > - > __current_set_polling(); > quiet_vmstat(); > tick_nohz_idle_enter(); > + cpuidle_entry_end(); > > while (!need_resched()) { > check_pgt_cache(); >
