From: John Kacur on


On Fri, 23 Apr 2010, Yong Zhang wrote:

> On Thu, Apr 22, 2010 at 10:15:48PM +0200, John Kacur wrote:
> > NOT FOR INCLUSION
> >
> > I created this patch as a result of Peter Zilstra's request to get more
> > info from lockdep. This patch is not for inclusion, at least in its
> > present form, because it adds some redunant info to /proc/lockdep_stats
> >
> > However, some of the fields are new, and it is worth examining, and / or
> > applying if you are looking at the MAX_STACK_TRACE_ENTRIES too big
> > problem.
> >
> > I generated this patch against a recent tip/master but it applies without
> > conflicts to the latest rt kernel as well. Comments are welcome, in fact
> > they are appreciated.
> >
> > >From 5181c0296dd1549e4e706ff25a4cd81a1d90137d Mon Sep 17 00:00:00 2001
> > From: John Kacur <jkacur(a)redhat.com>
> > Date: Thu, 22 Apr 2010 17:02:42 +0200
> > Subject: [PATCH] lockdep: Add nr_save_trace_invocations counter
> >
> > Add the nr_save_trace_invocations counter which counts the number of
> > time save_trace() is invoked when relevant for trace enteries.
> >
> > This means, those invocations from mark_lock() and add_lock_to_list()
> >
> > When called from mark_lock() we break it down into LOCKSTATE categories.
> >
> > Signed-off-by: John Kacur <jkacur(a)redhat.com>
>
> Just take a rough look at it. I don't think this can give more info.
>
> > +/* Calls to save_trace() from mark_lock() and add_lock_to_list() only*/
> > +unsigned long nr_save_trace_invocations;
>
> It will equal to nr_list_entries.
>
> > +unsigned long nr_save_trace_invocations_type[LOCK_USAGE_STATES];
>
> And each item in this array will equal to nr_hardirq_[un]safe,
> nr_softirq_[un]safe and such things under lockdep_stats_show(). Right?
>
> Thanks,
> Yong
>

Hi Yong

Some context here - Peter asked me to see if we could get some more
detailed stats on why some configurations reach the
MAX_STACK_TRACE_ENTRIES limit - whether the limit was really too low for
some circumstances, or whether we were counting somethings unnecessarily.

In any case, I stamped a big NOT FOR INCLUSION on my mail, because I
noticed that somethings were redundant - albeit, obtained in a slightly
different manner, however, not everything is redundant.

In particular, nr_save_trace_invocations is NOT equal to nr_list_entries.
You will see that reported in /proc/lockdep_stats as
direct dependencies: 8752 [max: 16384]
I have
stack-trace invocations: 10888
from the same run.

Still trying to figure out what the meaning is of that though to be
honest.

Here is a portion of the lockdep_stats, with all of the new fields and the
redundant ones.

stack-trace invocations: 10888
LOCK_USED_IN_HARDIRQ: 15
LOCK_USED_IN_HARDIRQ_READ: 0
LOCK_ENABLED_HARDIRQ: 543
LOCK_ENABLED_HARDIRQ_READ: 28
LOCK_USED_IN_SOFTIRQ: 0
LOCK_USED_IN_SOFTIRQ_READ: 0
LOCK_ENABLED_SOFTIRQ: 543
LOCK_ENABLED_SOFTIRQ_READ: 28
LOCK_USED_IN_RECLAIM_FS: 5
LOCK_USED_IN_RECLAIM_FS_READ: 0
LOCK_ENABLED_RECLAIM_FS: 95
LOCK_ENABLED_RECLAIM_FS_READ: 8
LOCK_USED: 871
combined max dependencies: 139841
hardirq-safe locks: 15
hardirq-unsafe locks: 543
softirq-safe locks: 0
softirq-unsafe locks: 543
irq-safe locks: 15
irq-unsafe locks: 543
hardirq-read-safe locks: 0
hardirq-read-unsafe locks: 28
softirq-read-safe locks: 0
softirq-read-unsafe locks: 28
irq-read-safe locks: 0
irq-read-unsafe locks: 28

So, you see that all of the reclaim fields are new,
LOCK_USED_IN_RECLAIM_FS: 5
LOCK_USED_IN_RECLAIM_FS_READ: 0
LOCK_ENABLED_RECLAIM_FS: 95
LOCK_ENABLED_RECLAIM_FS_READ: 8

I can create a patch for inclusion that adds the reclaim fields, the
question is, is the nr_save_trace_invocations a useful stat for us or not?

Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Peter Zijlstra on
On Fri, 2010-04-23 at 09:24 +0200, John Kacur wrote:
> direct dependencies: 8752 [max: 16384]
> I have
> stack-trace invocations: 10888
> from the same run.
>
> Still trying to figure out what the meaning is of that though to be
> honest.
>
> Here is a portion of the lockdep_stats, with all of the new fields and the
> redundant ones.
>
> stack-trace invocations: 10888
> LOCK_USED_IN_HARDIRQ: 15
> LOCK_USED_IN_HARDIRQ_READ: 0
> LOCK_ENABLED_HARDIRQ: 543
> LOCK_ENABLED_HARDIRQ_READ: 28
> LOCK_USED_IN_SOFTIRQ: 0
> LOCK_USED_IN_SOFTIRQ_READ: 0
> LOCK_ENABLED_SOFTIRQ: 543
> LOCK_ENABLED_SOFTIRQ_READ: 28
> LOCK_USED_IN_RECLAIM_FS: 5
> LOCK_USED_IN_RECLAIM_FS_READ: 0
> LOCK_ENABLED_RECLAIM_FS: 95
> LOCK_ENABLED_RECLAIM_FS_READ: 8
> LOCK_USED: 871

8752+871+8+95+5+28+543+28+543+15=10888

So you get a stack-trace for each direct-dependency, and you get a
stack-trace for each LOCK_state, the sum seems to match the total
invocations.

Non of these numbers look strange..

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yong Zhang on
On Fri, Apr 23, 2010 at 09:24:55AM +0200, John Kacur wrote:
> Some context here - Peter asked me to see if we could get some more
> detailed stats on why some configurations reach the
> MAX_STACK_TRACE_ENTRIES limit - whether the limit was really too low for
> some circumstances, or whether we were counting somethings unnecessarily.
>
> In any case, I stamped a big NOT FOR INCLUSION on my mail, because I
> noticed that somethings were redundant - albeit, obtained in a slightly
> different manner, however, not everything is redundant.
>
> In particular, nr_save_trace_invocations is NOT equal to nr_list_entries.
> You will see that reported in /proc/lockdep_stats as
> direct dependencies: 8752 [max: 16384]
> I have
> stack-trace invocations: 10888
> from the same run.

I missed that nr_save_trace_invocations is also increased in
inc_save_trace_invocations().
So nr_save_trace_invocations = nr_list_entries + sum of
nr_save_trace_invocations_type[].

>
> Still trying to figure out what the meaning is of that though to be
> honest.
>
> Here is a portion of the lockdep_stats, with all of the new fields and the
> redundant ones.
>
> stack-trace invocations: 10888
> LOCK_USED_IN_HARDIRQ: 15
> LOCK_USED_IN_HARDIRQ_READ: 0
> LOCK_ENABLED_HARDIRQ: 543
> LOCK_ENABLED_HARDIRQ_READ: 28
> LOCK_USED_IN_SOFTIRQ: 0
> LOCK_USED_IN_SOFTIRQ_READ: 0
> LOCK_ENABLED_SOFTIRQ: 543
> LOCK_ENABLED_SOFTIRQ_READ: 28
> LOCK_USED_IN_RECLAIM_FS: 5
> LOCK_USED_IN_RECLAIM_FS_READ: 0
> LOCK_ENABLED_RECLAIM_FS: 95
> LOCK_ENABLED_RECLAIM_FS_READ: 8
> LOCK_USED: 871
> combined max dependencies: 139841
> hardirq-safe locks: 15
> hardirq-unsafe locks: 543
> softirq-safe locks: 0
> softirq-unsafe locks: 543
> irq-safe locks: 15
> irq-unsafe locks: 543
> hardirq-read-safe locks: 0
> hardirq-read-unsafe locks: 28
> softirq-read-safe locks: 0
> softirq-read-unsafe locks: 28
> irq-read-safe locks: 0
> irq-read-unsafe locks: 28
>
> So, you see that all of the reclaim fields are new,
> LOCK_USED_IN_RECLAIM_FS: 5
> LOCK_USED_IN_RECLAIM_FS_READ: 0
> LOCK_ENABLED_RECLAIM_FS: 95
> LOCK_ENABLED_RECLAIM_FS_READ: 8

Yes, indeed, data in lockdep_stats_show() is out of time.
So as Peter has said in another thread, we should add sample for RECLAIM_FS.

>
> I can create a patch for inclusion that adds the reclaim fields, the
> question is, is the nr_save_trace_invocations a useful stat for us or not?

Actually it's just a summation of the samples.
I don't think it's necessary.

Thanks,
Yong
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yong Zhang on
On Fri, Apr 23, 2010 at 08:52:05AM +0200, Peter Zijlstra wrote:
> On Fri, 2010-04-23 at 10:58 +0800, Yong Zhang wrote:
> >
> > > +unsigned long nr_save_trace_invocations_type[LOCK_USAGE_STATES];
> >
> > And each item in this array will equal to nr_hardirq_[un]safe,
> > nr_softirq_[un]safe and such things under lockdep_stats_show(). Right?
>
> Well, the stats for RECLAIM_FS as missing, so at the very least we need
> to re-write lockdep_stats_show() to use the lockdep_states.h magic.

Yeah, it's a good start point and will keep thing unifying in the future.

Thanks,
Yong
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/