> > 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
