Messages in this thread | | | Date | Tue, 2 Feb 2016 20:15:14 +0530 | From | Srikar Dronamraju <> | Subject | Re: [PATCH 1/1] sched: Make schedstats a runtime tunable that is disabled by default v3 |
| |
> > If we disable schedstats dynamically but CONFIG_TASK_DELAY_ACCT is not > > set, then sched_info_on will return true, > > Is this really a problem? > > sched_info_on() guards sched_info_dequeued(), sched_info_queued(), > __sched_info_switch(). These update fields in the sched_info struct with > the exception of rq->rq_cpu_time. In the case of rq_cpu_time, the values > it's updated depend on sched_info. > > I'm not spotting the case where the current information for delayacct is > inaccurate. Where is it? Granted, there is some scope for also disabling > the delayacct information unless explicitly enabled.
ah, the run_delay gets updated. So yes its not a problem.
> > > This could impact guest steal > > time stats as well as data read from /proc/<PID>/schedstat > > > > Also when schedstats is dynamically disabled, and user tries to enable > > kernel sleep profiling profile_setup(), the kernel may not be able to do > > the right profiling since enqueue_sleeper() may not get called. Should > > we alert the user saying kernel sleep profiling is disabled? > > > > Yes. This on top? It's not completely bullet proof as a user could both > force schedstat disabled and enable sleep profiling but it's a waste of > memory to guard against it > > diff --git a/include/linux/sched.h b/include/linux/sched.h > index a10494a94cc3..5c2cd37c42e9 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -920,6 +920,14 @@ static inline int sched_info_on(void) > #endif > } > > +#ifdef CONFIG_SCHEDSTATS > +void force_schedstat_enabled(void); > +#else > +static inline void force_schedstat_enabled(void) > +{ > +} > +#endif
One nit: Since force_schedstat_enabled is called under CONFIG_SCHEDSTATS we may not want the static define.
> + > enum cpu_idle_type { > CPU_IDLE, > CPU_NOT_IDLE, > diff --git a/kernel/profile.c b/kernel/profile.c > index 99513e1160e5..51369697466e 100644 > --- a/kernel/profile.c > +++ b/kernel/profile.c > @@ -59,6 +59,7 @@ int profile_setup(char *str) > > if (!strncmp(str, sleepstr, strlen(sleepstr))) { > #ifdef CONFIG_SCHEDSTATS > + force_schedstat_enabled(); > prof_on = SLEEP_PROFILING; > if (str[strlen(sleepstr)] == ',') > str += strlen(sleepstr) + 1;
-- Thanks and Regards Srikar Dronamraju
| |