[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 01 Jan 2010 16:10:16 +0100
From: Stefani Seibold <stefani@...bold.net>
To: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Cc: LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Alexey Dobriyan <adobriyan@...il.com>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Randy Dunlap <randy.dunlap@...cle.com>,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] proc: revert to show stack information in
/proc/{pid}/status
Am Freitag, den 01.01.2010, 23:14 +0900 schrieb KOSAKI Motohiro:
> > Am Donnerstag, den 31.12.2009, 23:12 +0900 schrieb KOSAKI Motohiro:
> > > Commit d899bf7b (procfs: provide stack information for threads) introduced
> > > to show stack information in /proc/{pid}/status. But it cause large performance
> > > regression. Unfortunately /proc/{pid}/status is used ps command too and ps is
> > > one of most important component. Because both to take mmap_sem and page table walk
> > > are heavily operation.
> > >
> >
> > /proc/<pid>/status is IMHO not a performance relevant thing. The main
> > reason is to provide exact information about a running process.
>
> No. You have to learn real world use case. if you think so, you should
> change ps before submit this change. This patch obviously make harm than worth.
> Nobody (except you) use it but everybody get regression.
>
It is fascinating that every developer means that only his personal view
and requirements are the wisdom of the world.
>
> > > Thus, this patch revert it. Fortunately /proc/{pid}/task/{tid}/smaps
> > > provide almost same information. we can use it.
> >
> > Completely wrong. Where does smaps provides this kind of information?
> > Where is there the high water mark of the stack usage?
>
> You have to see you patch itself. show_map_vma() isn't only used by /proc/pid/maps,
> but also be used by /proc/pid/smaps.
>
> Now, /proc/{pid}/task/{tid}/smaps show following column.
>
> 7fb97c181000-7fb97d1d1000 rw-p 00000000 00:00 0 [threadstack:0084eff0]
> Size: 16704 kB
> Rss: 8 kB
> Pss: 8 kB
> Shared_Clean: 0 kB
> Shared_Dirty: 0 kB
> Private_Clean: 0 kB
> Private_Dirty: 8 kB
> Referenced: 8 kB
> Swap: 0 kB
> KernelPageSize: 4 kB
> MMUPageSize: 4 kB
>
> It descibe
> - This vma is thread stack
> - vma size is 16704kB
> - stack vsz size in vma is 0x0084eff0 (~ 8508kB)
> - and, physical memory is used 8kB.
>
But it don't describe the usage high water mark. With the information
provided by proc/*/smaps this is not possible. It is very funny to get
complains without checking. Your assertion is completely WRONG.
>
> Anyway, I revert the regresstion patch as other regression patches. if you really want
> this feature, you have three options.
>
> 1. create new /proc file instead to use /proc/pid/status.
This was discarded by Andrew. He prefered the inclusion
in /proc/pid/status.
> 2. improve performance until typical use-case don't notice regression.
Not possible.
> 3. change ps and other /proc related userland implementation and resubmit this patch.
ps works quiet well.
>
> But even if you choose anything, You have to test both its functional and performance
> _before_ submitting kernel patch.
Teach me master! Do you think i don't know that is coast something?.
Walking through the pages coast some runtime. But ps is not a
performance critical application nor a cat /proc/*/status!
Stefani
--
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/
Powered by blists - more mailing lists