linux-kernel - Re: [PATCH] proc/softirqs: only show state for online cpus

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM2zO=CxriXK4bqGbwhfkfdgSpGwe1mAJE=6cYpDAf0Hxji0sQ@mail.gmail.com>
Date:	Tue, 26 Jul 2011 15:29:43 +0800
From:	Yong Zhang <yong.zhang0@...il.com>
To:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Cc:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	kobayashi.kk@...s.nec.co.jp
Subject: Re: [PATCH] proc/softirqs: only show state for online cpus

On Tue, Jul 26, 2011 at 2:38 PM, KOSAKI Motohiro
<kosaki.motohiro@...fujitsu.com> wrote:
> (2011/07/26 15:14), Yong Zhang wrote:
>> 2011/7/26 KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>:
>>>> Like /proc/interrupts, no need to output data for nobody.
>>>>
>>>> Signed-off-by: Yong Zhang <yong.zhang0@...il.com>
>>>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>>>> Cc: Keika Kobayashi <kobayashi.kk@...s.nec.co.jp>
>>>> Cc: KOSAKI Motohiro <kosaki.motohiro@...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.

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


-- 
Only stand for myself
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/