MIME-Version: 1.0
In-Reply-To: <20170809074337.awcwac6u2cxr5h2m@hirez.programming.kicks-ass.net>
References: <1502274688-2508-1-git-send-email-laoar.shao@gmail.com> <20170809074337.awcwac6u2cxr5h2m@hirez.programming.kicks-ass.net>
From: Yafang Shao <laoar.shao@gmail.com>
Date: Wed, 9 Aug 2017 16:01:49 +0800
Message-ID: <CALOAHbDbfRkexFsyxtRjky1zkLNu_5KMTZhMoDStjjH3yvi6OA@mail.gmail.com>
Subject: Re: [PATCH v3] scheduler: enhancement to show_state_filter and SysRq
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@redhat.com, gregkh@linuxfoundation.org, jslaby@suse.com,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2331
Lines: 58

2017-08-09 15:43 GMT+08:00 Peter Zijlstra <peterz@infradead.org>:
> On Wed, Aug 09, 2017 at 06:31:28PM +0800, Yafang Shao wrote:
>> Sometimes we want to get tasks in TASK_RUNNING sepcifically,
>> instead of dump all tasks.
>>
>> For example, when the loadavg are high, we want to dump
>> tasks in TASK_RUNNING and TASK_UNINTERRUPTIBLE, which contribute
>> to system load. But mostly there're lots of tasks in Sleep state,
>> which occupies almost all of the kernel log buffer, even overflows
>> it, that causes the useful messages get lost. Although we can
>> enlarge the kernel log buffer, but that's not a good idea.
>
> That's what you have serial consoles for...
>
mostly we don't even have one console because we alwayas login the
servers via ssh. And manage the servers with console is not so convenient.


>> +static void sysrq_handle_showstate_load(int key)
>>  {
>> +     show_state_filter(TASK_UNINTERRUPTIBLE << 1 | (TASK_RUNNING | 0x1));
>>  }
>
> How is that not unreadable gunk?

I know it is very hard to read, that's why I put TASK_RUNNING here.
But in order to Backward Compatibility, I have to do it like this.

>
>> @@ -477,7 +478,7 @@ static void sysrq_handle_unrt(int key)
>>       &sysrq_mountro_op,              /* u */
>>       /* v: May be registered for frame buffer console restore */
>>       NULL,                           /* v */
>> -     &sysrq_showstate_blocked_op,    /* w */
>> +     &sysrq_showstate_load_op,       /* w */
>>       /* x: May be registered on mips for TLB dump */
>>       /* x: May be registered on ppc/powerpc for xmon */
>>       /* x: May be registered on sparc64 for global PMU dump */
>
> So I'm really not convinced this is useful. The blocked thing is very
> useful if you're trying to debug a deadlock. Now you get endless clutter
> with runnable tasks.
>
> High load-avg as such isn't a problem. Why do you care?

Lots of tasks in blocked state, may not mean deadlock.
In most cases, it means tasks blocked in Disk-IO or some other
hardware resources.
So, when lots of tasks in blocked state, we need to know which
processes are running and holding
the lock, and what thses processes running for.  high load is one
example of these cases.
As tasks in blocked state and running state contribute to system load,
that's why I
name the function with 'load'.

Thanks
Yafang