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

[Yong Zhang]

On Tue, Jul 26, 2011 at 2:38 PM, KOSAKI Motohiro
<kosaki.motohiro@xxxxxxxxxxxxxx> wrote:
> (2011/07/26 15:14), Yong Zhang wrote:
>> 2011/7/26 KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>:
>>>> Like /proc/interrupts, no need to output data for nobody.
>>>>
>>>> Signed-off-by: Yong Zhang <yong.zhang0@xxxxxxxxx>
>>>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>>>> Cc: Keika Kobayashi <kobayashi.kk@xxxxxxxxxxxxxx>
>>>> Cc: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>
>>>
>>> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/