Messages in this thread | | | Date | Tue, 26 May 2009 22:46:49 +0200 | From | Frederic Weisbecker <> | Subject | Re: [PATCH 1/3] tracing/stat: sort in ascending order |
| |
On Tue, May 26, 2009 at 09:09:24AM +0800, Li Zefan wrote: > Frederic Weisbecker wrote: > > On Mon, May 25, 2009 at 04:46:09PM +0800, Li Zefan wrote: > >> Currently the output of trace_stat/workqueues is totally reversed: > >> > >> # cat /debug/tracing/trace_stat/workqueues > >> ... > >> 1 17 17 210 37 `-blk_unplug_work+0x0/0x57 > >> 1 3779 3779 181 11 |-cfq_kick_queue+0x0/0x2f > >> 1 3796 3796 kblockd/1:120 > >> ... > >> > >> The correct output should be: > >> > >> 1 3796 3796 kblockd/1:120 > >> 1 3779 3779 181 11 |-cfq_kick_queue+0x0/0x2f > >> 1 17 17 210 37 `-blk_unplug_work+0x0/0x57 > >> > >> It's caused by "tracing/stat: replace linked list by an rbtree for sorting" > >> (53059c9b67a62a3dc8c80204d3da42b9267ea5a0). > >> > >> Though we can simply change dummy_cmp() to return -1 instead of 1, IMO > >> it's better to always do ascending sorting in trace_stat.c, and leave each > >> stat tracer to decide whether to sort in descending or ascending order. > >> > >> [ Impact: fix the output of trace_stat/workqueue ] > >> > >> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> > > > > > > For now in stat tracing, the ascendent sorting is the most relevant. > > Especially because we always want to see the highest problems first. > > > > Yeah, I saw this. > > > -1 (or < 0) usually means lower and 1 ( > 0) is higher. > > > > I wonder what would most confuse the developers of stat tracers: > > > > - to reverse these common sort values (-1 turn into "higher") > > - keep the default ascendent sorting, which is not natural because the default > > is often descendent. > > > > I don't know. > > > > Anyone else. Do you have a preference? > > > > When I looked into this bug, I was confused why it's descending sorting, though > then I found out the answer. > > Imagine a new stat tracer wants to sort in ascending order, but it has to define > a cmp which compares in reverse order. This seems to be odd and confusing. > > But to be honest, I'm also not sure which is better, And it doesn't seem to be > a big issue. So I think I'll make concessions.
Ok, so let's keep it as is and we'll see if developers complain :)
| |