On 2015/8/4 2:43, Peter Zijlstra wrote:
> On Mon, Aug 03, 2015 at 01:35:28PM -0400, Steven Rostedt wrote:
>> On Mon, 27 Jul 2015 09:11:52 -0400
>> yangoliver <yang_oli...@hotmail.com> wrote:
>>
>>> Per sched_stat_sleep definition in sched.h, it should include
>>> iowait case. This can also relect the design of sum_sleep_runtime
>>> statistic, as this counter also includes the io_wait.
>>>
>>> Signed-off-by: Yong Yang <yangoli...@gmail.com>
>>> ---
>>> kernel/sched/fair.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>>> index d113c3b..85677bf 100644
>>> --- a/kernel/sched/fair.c
>>> +++ b/kernel/sched/fair.c
>>> @@ -3018,6 +3018,8 @@ static void enqueue_sleeper(struct cfs_rq *cfs_rq,
>>> struct sched_entity *se)
>>> se->statistics.sum_sleep_runtime += delta;
>>>
>>> if (tsk) {
>>> + trace_sched_stat_sleep(tsk, delta);
>>> +
>>> if (tsk->in_iowait) {
>>> se->statistics.iowait_sum += delta;
>>> se->statistics.iowait_count++;
> No, that's broken in two ways. Firstly you don't change semantics of
> stuff just because of a comment and secondly iowait has nothing what all
> to do with INTERRUPTIBLE/sleep vs UNINTERRUPTIBLE/blocked.
Peter,
Sorry for missing key person in this mail thread.
Another reason I think sched_stat_sleep should cover UNINTERRUPTIBLE/blocked
case
is, the sum_sleep_runtime counter get increased for both INTERRUPTIBLE and
UNINTERRUPTIBLE cases. We can find below statement for both cases in the code,
se->statistics.sum_sleep_runtime += delta;
Plus below comments, I guessed the sched_stat_sleep trace point is originally
designed for cover all kind of sleep cases: interruptible and uninterruptible,
/*
* Tracepoint for accounting sleep time (time the task is not runnable,
* including iowait, see below).
*/
DEFINE_EVENT(sched_stat_template, sched_stat_sleep,
TP_PROTO(struct task_struct *tsk, u64 delay),
TP_ARGS(tsk, delay));
Do you think we should make sched_stat_sleep meaning similar with the
sum_sleep_runtime counter?
If not, we may need fix the comments in sched.h above.
>
> And wtf are you doing sending sched patches and not Cc maintainers.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
