lkml.org 
[lkml]   [2011]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] proc/softirqs: only show state for online cpus
(2011/07/26 16:29), Yong Zhang wrote:
> On Tue, Jul 26, 2011 at 2:38 PM, KOSAKI Motohiro
> <kosaki.motohiro@jp.fujitsu.com> wrote:
>> (2011/07/26 15:14), Yong Zhang wrote:
>>> 2011/7/26 KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>:
>>>>> Like /proc/interrupts, no need to output data for nobody.
>>>>>
>>>>> Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
>>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>>> Cc: Keika Kobayashi <kobayashi.kk@ncos.nec.co.jp>
>>>>> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>>>>
>>>> If the cpu never be onlined, its statistics always 0. Then, it definitely
>>>
>>> Yeah, so your screen may contain noise.
>>
>> One question. Is this big matter?
>
> Actually it doesn't :)
>
>> Who see /proc/softirqs and /proc/interrupts directly? (i.e. by 'cat' command).
>
> By accident I noticed it by accident when running rt kernel. My screen
> is full of '0'.
> You know my usage is just for testing, maybe the real user is script-like.
>
>>
>>
>>>> no value. In the other hand, if the cpu was offlined dynamically, we don't
>>>> know the user want to know the cpus's statistics or not.
>>>
>>> Same to /proc/interrupts :)
>>>
>>> IMHO, if user want to check the value of offline-cpu, maybe that means
>>> he want to check the state of the whole system, /proc/stat should be the
>>> right choice. /proc/{softirqs,interrupts} is just for immediate state.
>>>
>>>> Anyway, it's incompatibility change.
>>>
>>> Yup, I should have marked the patch with RFC :)
>>
>> And I should have remarked I don't dislike this patch so strongly, so
>> if kobayashi-san who original /proc/softirqs author ack you, I'm going
>> to second him.
>
> Hmmm, so let kobayashi-san decide it.

for_each_online_cpu() was in my first patch, like /proc/softirq.

But Andrew said
--
Probably for_each_possible_cpu() is best - people might want to see how
many softirqs happened on a CPU which was recently offlined.
--
It makes sense.

We would like to collect this information
for trouble-shooting.
I think for_each_possible_cpu() is better.

At that time, I suggested to change
from for_each_online_cpu() to for_each_possible_cpu(),
in /proc/interrupts.

In conclusion, we decided to remain /proc/interrupts.
because it had been the way for a long time.



>> Offtopic, /proc/interrupt should be protected by get_online_cpus().
>> Otherwise the header (i.e. cpu number) and the actual statistics fields
>> can be mismatched likes following. Am I missing something?
>
> I think you are right. The reader could be preempted by cpu hotplug.
>
> After searching the whole tree, only s390 take cpu_hotplug.lock,
> but its usage is not currect:
>
> arch/s390/kernel/irq.c:
> int show_interrupts(struct seq_file *p, void *v)
> {
> get_online_cpus();
> .........
> put_online_cpus();
> }
>
> Because the reader will call show_interrupts nr_irqs times.
> So get_online_cpus()/put_online_cpus() should be put upper,
> maybe interrupts_open(). How do you think about it?
>
> Thanks,
> Yong
>
>


--
――――――――――――――――――――――
NEC通信システム
技術管理本部 Linux技術センター
小林 恵果
Mail : kobayashi.kk@ncos.nec.co.jp
Tel : 04-7185-6956(内線 : 8-26-35686)
――――――――――――――――――――――
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2011-07-26 09:59    [W:1.763 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site