Peter Zijlstra wrote:
On Wed, 2007-06-06 at 23:34 +0200, Martin Peschke wrote:
+#ifdef CONFIG_LOCK_STAT
+enum lock_stat_enum {
+ LOCK_STAT_CONT,
+ LOCK_STAT_WAIT_READ,
+ LOCK_STAT_WAIT_WRITE,
+ LOCK_STAT_HOLD_READ,
+ LOCK_STAT_HOLD_WRITE,
+ _LOCK_STAT_NUMBER
Bill Huey (hui) wrote:
> However, I don't understand why all of this
> is so heavy weight when the current measurements that Peter makes is
> completely sufficient for any reasonable purpose I can think of at the
> moment.
It wasn't meant to be or to stay heavy weight. To make the best of my
Peter Zijlstra wrote:
On Fri, 2007-06-08 at 19:37 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
On Fri, 2007-06-08 at 19:18 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
On Fri, 2007-06-08 at 19:00 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
I'm confused as to where the
Peter Zijlstra wrote:
On Fri, 2007-06-08 at 19:37 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
On Fri, 2007-06-08 at 19:18 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
On Fri, 2007-06-08 at 19:00 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
I'm confused as to where the
Bill Huey (hui) wrote:
However, I don't understand why all of this
is so heavy weight when the current measurements that Peter makes is
completely sufficient for any reasonable purpose I can think of at the
moment.
It wasn't meant to be or to stay heavy weight. To make the best of my
recent
Peter Zijlstra wrote:
On Wed, 2007-06-06 at 23:34 +0200, Martin Peschke wrote:
+#ifdef CONFIG_LOCK_STAT
+enum lock_stat_enum {
+ LOCK_STAT_CONT,
+ LOCK_STAT_WAIT_READ,
+ LOCK_STAT_WAIT_WRITE,
+ LOCK_STAT_HOLD_READ,
+ LOCK_STAT_HOLD_WRITE,
+ _LOCK_STAT_NUMBER
On Fri, 2007-06-08 at 19:37 +0200, Martin Peschke wrote:
> Peter Zijlstra wrote:
> > On Fri, 2007-06-08 at 19:18 +0200, Martin Peschke wrote:
> >> Peter Zijlstra wrote:
> >>> On Fri, 2007-06-08 at 19:00 +0200, Martin Peschke wrote:
> Peter Zijlstra wrote:
> > I'm confused as to where the
Peter Zijlstra wrote:
On Fri, 2007-06-08 at 19:18 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
On Fri, 2007-06-08 at 19:00 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
I'm confused as to where the class->stat objects are initialised? Is
that done in lock_stat_init()? If so, then
On Fri, 2007-06-08 at 19:18 +0200, Martin Peschke wrote:
> Peter Zijlstra wrote:
> > On Fri, 2007-06-08 at 19:00 +0200, Martin Peschke wrote:
> >> Peter Zijlstra wrote:
> >>> I'm confused as to where the class->stat objects are initialised? Is
> >>> that done in lock_stat_init()? If so, then you
Peter Zijlstra wrote:
On Wed, 2007-06-06 at 23:34 +0200, Martin Peschke wrote:
+struct statistic_info lock_stat_info[_LOCK_STAT_NUMBER] = {
+ [LOCK_STAT_CONT] = {
+ .name = "contentions",
+ .x_unit = "instruction_pointer",
+ .y_unit =
Peter Zijlstra wrote:
I'm confused as to where the class->stat objects are initialised? Is
that done in lock_stat_init()? If so, then you have a bug.
static struct lock_class lock_classes[MAX_LOCKDEP_KEYS];
I assume this gets us class structures containing all zeros, doesn't it?
Then
Ingo Molnar wrote:
> Firstly, submit cleanup patches that _do not change the output_. If you
> have any output changes, do it as a separate patch, ontop of the cleanup
> patch. Mixing material changes and cleanups into a single patch is a
> basic patch submission mistake that will only earn you
Ingo Molnar wrote:
if the infrastructure your are advocating does not allow us to keep the
existing output then it's simply not flexible enough.
Let's be precise. If "keep the existing output" means any format change is
unacceptible to you, then I broke things. If it means that my method
Ingo Molnar wrote:
if the infrastructure your are advocating does not allow us to keep the
existing output then it's simply not flexible enough.
Let's be precise. If keep the existing output means any format change is
unacceptible to you, then I broke things. If it means that my method
Ingo Molnar wrote:
Firstly, submit cleanup patches that _do not change the output_. If you
have any output changes, do it as a separate patch, ontop of the cleanup
patch. Mixing material changes and cleanups into a single patch is a
basic patch submission mistake that will only earn you
Peter Zijlstra wrote:
On Wed, 2007-06-06 at 23:34 +0200, Martin Peschke wrote:
+struct statistic_info lock_stat_info[_LOCK_STAT_NUMBER] = {
+ [LOCK_STAT_CONT] = {
+ .name = contentions,
+ .x_unit = instruction_pointer,
+ .y_unit =
Peter Zijlstra wrote:
I'm confused as to where the class-stat objects are initialised? Is
that done in lock_stat_init()? If so, then you have a bug.
static struct lock_class lock_classes[MAX_LOCKDEP_KEYS];
I assume this gets us class structures containing all zeros, doesn't it?
Then
Peter Zijlstra wrote:
On Fri, 2007-06-08 at 19:18 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
On Fri, 2007-06-08 at 19:00 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
I'm confused as to where the class-stat objects are initialised? Is
that done in lock_stat_init()? If so, then
On Fri, 2007-06-08 at 19:18 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
On Fri, 2007-06-08 at 19:00 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
I'm confused as to where the class-stat objects are initialised? Is
that done in lock_stat_init()? If so, then you have a bug.
On Fri, 2007-06-08 at 19:37 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
On Fri, 2007-06-08 at 19:18 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
On Fri, 2007-06-08 at 19:00 +0200, Martin Peschke wrote:
Peter Zijlstra wrote:
I'm confused as to where the class-stat objects
* Peter Zijlstra <[EMAIL PROTECTED]> wrote:
> we have around 1400 locks in the kernel, this would give 414400 per cpu.
>
> vs the old code:
>
> 2048*(4*8) = 65536
> +
> 2048*(4*4*8 + 4*8) = 327680 per cpu
>
> worst case
>
> I'm not convinced 300 lines less code is worth that extra bloat.
On Thu, Jun 07, 2007 at 09:30:21AM +0200, Ingo Molnar wrote:
> * Martin Peschke <[EMAIL PROTECTED]> wrote:
> > Do mean I might submit this stuff for -rt?
>
> Firstly, submit cleanup patches that _do not change the output_. If you
> have any output changes, do it as a separate patch, ontop of the
On Wed, 2007-06-06 at 23:34 +0200, Martin Peschke wrote:
> +#ifdef CONFIG_LOCK_STAT
> +enum lock_stat_enum {
> + LOCK_STAT_CONT,
> + LOCK_STAT_WAIT_READ,
> + LOCK_STAT_WAIT_WRITE,
> + LOCK_STAT_HOLD_READ,
> + LOCK_STAT_HOLD_WRITE,
> + _LOCK_STAT_NUMBER
> +};
> +#endif
> +
On Wed, 2007-06-06 at 23:34 +0200, Martin Peschke wrote:
> +struct statistic_info lock_stat_info[_LOCK_STAT_NUMBER] = {
> + [LOCK_STAT_CONT] = {
> + .name = "contentions",
> + .x_unit = "instruction_pointer",
> + .y_unit = "occurrence",
> +
On Wed, 2007-06-06 at 23:34 +0200, Martin Peschke wrote:
>
> Signed-off-by: Martin Peschke <[EMAIL PROTECTED]>
> ---
>
> include/linux/lockdep.h | 35 ++---
> kernel/lockdep.c| 122 ++-
> kernel/lockdep_proc.c | 294
>
* Martin Peschke <[EMAIL PROTECTED]> wrote:
> Bill Huey (hui) wrote:
> > There are roughly about 400 locks in a normal kernel for a desktop. The
> > list is rather cumbersome anyways so, IMO, it really should be handled
> > by parsing tools, etc... There could be more properties attached to each
* Martin Peschke <[EMAIL PROTECTED]> wrote:
> Admittedly this gives you the top five contention points, [...]
if the infrastructure your are advocating does not allow us to keep the
existing output then it's simply not flexible enough. Why on earth are
you even arguing about this? A "cleanup"
Bill Huey (hui) wrote:
> There are roughly about 400 locks in a normal kernel for a desktop. The
> list is rather cumbersome anyways so, IMO, it really should be handled
> by parsing tools, etc... There could be more properties attached to each
> lock especially if you intend to get this to work
Peter Zijlstra wrote:
On Thu, 2007-06-07 at 02:17 +0200, Martin Peschke wrote:
Ingo Molnar wrote:
* Martin Peschke <[EMAIL PROTECTED]> wrote:
The output has changed from a terribly wide table to an enormously
long list (just the generic way the statistics code prints data).
Sigh, why dont
On Thu, 2007-06-07 at 02:17 +0200, Martin Peschke wrote:
> Ingo Molnar wrote:
> > * Martin Peschke <[EMAIL PROTECTED]> wrote:
> >
> >> The output has changed from a terribly wide table to an enormously
> >> long list (just the generic way the statistics code prints data).
> >
> > Sigh, why
On Thu, 2007-06-07 at 02:17 +0200, Martin Peschke wrote:
Ingo Molnar wrote:
* Martin Peschke [EMAIL PROTECTED] wrote:
The output has changed from a terribly wide table to an enormously
long list (just the generic way the statistics code prints data).
Sigh, why dont you _ask_
Peter Zijlstra wrote:
On Thu, 2007-06-07 at 02:17 +0200, Martin Peschke wrote:
Ingo Molnar wrote:
* Martin Peschke [EMAIL PROTECTED] wrote:
The output has changed from a terribly wide table to an enormously
long list (just the generic way the statistics code prints data).
Sigh, why dont
Bill Huey (hui) wrote:
There are roughly about 400 locks in a normal kernel for a desktop. The
list is rather cumbersome anyways so, IMO, it really should be handled
by parsing tools, etc... There could be more properties attached to each
lock especially if you intend to get this to work on
* Martin Peschke [EMAIL PROTECTED] wrote:
Admittedly this gives you the top five contention points, [...]
if the infrastructure your are advocating does not allow us to keep the
existing output then it's simply not flexible enough. Why on earth are
you even arguing about this? A cleanup
* Martin Peschke [EMAIL PROTECTED] wrote:
Bill Huey (hui) wrote:
There are roughly about 400 locks in a normal kernel for a desktop. The
list is rather cumbersome anyways so, IMO, it really should be handled
by parsing tools, etc... There could be more properties attached to each
lock
On Wed, 2007-06-06 at 23:34 +0200, Martin Peschke wrote:
Signed-off-by: Martin Peschke [EMAIL PROTECTED]
---
include/linux/lockdep.h | 35 ++---
kernel/lockdep.c| 122 ++-
kernel/lockdep_proc.c | 294
+++-
3
On Wed, 2007-06-06 at 23:34 +0200, Martin Peschke wrote:
+struct statistic_info lock_stat_info[_LOCK_STAT_NUMBER] = {
+ [LOCK_STAT_CONT] = {
+ .name = contentions,
+ .x_unit = instruction_pointer,
+ .y_unit = occurrence,
+
On Wed, 2007-06-06 at 23:34 +0200, Martin Peschke wrote:
+#ifdef CONFIG_LOCK_STAT
+enum lock_stat_enum {
+ LOCK_STAT_CONT,
+ LOCK_STAT_WAIT_READ,
+ LOCK_STAT_WAIT_WRITE,
+ LOCK_STAT_HOLD_READ,
+ LOCK_STAT_HOLD_WRITE,
+ _LOCK_STAT_NUMBER
+};
+#endif
+
/*
*
On Thu, Jun 07, 2007 at 09:30:21AM +0200, Ingo Molnar wrote:
* Martin Peschke [EMAIL PROTECTED] wrote:
Do mean I might submit this stuff for -rt?
Firstly, submit cleanup patches that _do not change the output_. If you
have any output changes, do it as a separate patch, ontop of the cleanup
* Peter Zijlstra [EMAIL PROTECTED] wrote:
we have around 1400 locks in the kernel, this would give 414400 per cpu.
vs the old code:
2048*(4*8) = 65536
+
2048*(4*4*8 + 4*8) = 327680 per cpu
worst case
I'm not convinced 300 lines less code is worth that extra bloat.
agreed, this is
On Thu, Jun 07, 2007 at 02:17:45AM +0200, Martin Peschke wrote:
> Ingo Molnar wrote:
> >, quite some work went into it - NACK :-(
>
> Considering the amount of code.. ;-)I am sorry.
>
> But seriously, did you consider using some user space tool or script to
> format this stuff the way you
Ingo Molnar wrote:
* Martin Peschke <[EMAIL PROTECTED]> wrote:
- lock_time_inc() vs. statistic_add_util()
please fix the coding style in lib/statistic.c. It's full of:
{
unsigned long long i;
if (value <= stat->u.histogram.range_min)
return 0;
put a newline
Ingo Molnar wrote:
* Martin Peschke <[EMAIL PROTECTED]> wrote:
The output has changed from a terribly wide table to an enormously
long list (just the generic way the statistics code prints data).
Sigh, why dont you _ask_ before doing such stuff?
A nice diffstat is always worth a try,
* Martin Peschke <[EMAIL PROTECTED]> wrote:
> - lock_time_inc() vs. statistic_add_util()
please fix the coding style in lib/statistic.c. It's full of:
{
unsigned long long i;
if (value <= stat->u.histogram.range_min)
return 0;
put a newline after variable
* Martin Peschke <[EMAIL PROTECTED]> wrote:
> The output has changed from a terribly wide table to an enormously
> long list (just the generic way the statistics code prints data).
Sigh, why dont you _ask_ before doing such stuff? It is a terribly wide
table because that makes it easily
This patch converts the lock statistics to a user of lib/statistic.c,
resulting in somewhat simpler code (a 300 lines-of-code diet).
Anyone interested in the details of code duplication might want to study:
- lock_time_inc() vs. statistic_add_util()
- lock_stats() vs. statistic_merge()
-
This patch converts the lock statistics to a user of lib/statistic.c,
resulting in somewhat simpler code (a 300 lines-of-code diet).
Anyone interested in the details of code duplication might want to study:
- lock_time_inc() vs. statistic_add_util()
- lock_stats() vs. statistic_merge()
-
* Martin Peschke [EMAIL PROTECTED] wrote:
The output has changed from a terribly wide table to an enormously
long list (just the generic way the statistics code prints data).
Sigh, why dont you _ask_ before doing such stuff? It is a terribly wide
table because that makes it easily
* Martin Peschke [EMAIL PROTECTED] wrote:
- lock_time_inc() vs. statistic_add_util()
please fix the coding style in lib/statistic.c. It's full of:
{
unsigned long long i;
if (value = stat-u.histogram.range_min)
return 0;
put a newline after variable sections.
Ingo Molnar wrote:
* Martin Peschke [EMAIL PROTECTED] wrote:
The output has changed from a terribly wide table to an enormously
long list (just the generic way the statistics code prints data).
Sigh, why dont you _ask_ before doing such stuff?
A nice diffstat is always worth a try, isn't
Ingo Molnar wrote:
* Martin Peschke [EMAIL PROTECTED] wrote:
- lock_time_inc() vs. statistic_add_util()
please fix the coding style in lib/statistic.c. It's full of:
{
unsigned long long i;
if (value = stat-u.histogram.range_min)
return 0;
put a newline
On Thu, Jun 07, 2007 at 02:17:45AM +0200, Martin Peschke wrote:
Ingo Molnar wrote:
, quite some work went into it - NACK :-(
Considering the amount of code.. ;-)I am sorry.
But seriously, did you consider using some user space tool or script to
format this stuff the way you like it -
52 matches
Mail list logo
