Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
* Peter Zijlstra wrote: > On Tue, Oct 02, 2018 at 10:10:48AM -0400, Waiman Long wrote: > > One alternative is to group it under CONFIG_DEBUG_LOCKDEP again. This > > metric was originally under CONFIG_DEBUG_LOCKDEP, but was moved to > > CONFIG_LOCKDEP when trying to make other lock debugging statistics > > per-cpu counters. It was probably because this metric is per lock class > > while the rests are global. > > > > By doing so, you incur the memory cost only when CONFIG_DEBUG_LOCKDEP is > > defined. > > > > What do you think? > > Works for me. Sounds good to me too! Thanks, Ingo
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
* Peter Zijlstra wrote: > On Tue, Oct 02, 2018 at 10:10:48AM -0400, Waiman Long wrote: > > One alternative is to group it under CONFIG_DEBUG_LOCKDEP again. This > > metric was originally under CONFIG_DEBUG_LOCKDEP, but was moved to > > CONFIG_LOCKDEP when trying to make other lock debugging statistics > > per-cpu counters. It was probably because this metric is per lock class > > while the rests are global. > > > > By doing so, you incur the memory cost only when CONFIG_DEBUG_LOCKDEP is > > defined. > > > > What do you think? > > Works for me. Sounds good to me too! Thanks, Ingo
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
On Tue, Oct 02, 2018 at 10:10:48AM -0400, Waiman Long wrote: > One alternative is to group it under CONFIG_DEBUG_LOCKDEP again. This > metric was originally under CONFIG_DEBUG_LOCKDEP, but was moved to > CONFIG_LOCKDEP when trying to make other lock debugging statistics > per-cpu counters. It was probably because this metric is per lock class > while the rests are global. > > By doing so, you incur the memory cost only when CONFIG_DEBUG_LOCKDEP is > defined. > > What do you think? Works for me.
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
On Tue, Oct 02, 2018 at 10:10:48AM -0400, Waiman Long wrote: > One alternative is to group it under CONFIG_DEBUG_LOCKDEP again. This > metric was originally under CONFIG_DEBUG_LOCKDEP, but was moved to > CONFIG_LOCKDEP when trying to make other lock debugging statistics > per-cpu counters. It was probably because this metric is per lock class > while the rests are global. > > By doing so, you incur the memory cost only when CONFIG_DEBUG_LOCKDEP is > defined. > > What do you think? Works for me.
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
On 10/02/2018 05:55 AM, Ingo Molnar wrote:
> * Peter Zijlstra wrote:
>
>> On Fri, Sep 28, 2018 at 01:53:20PM -0400, Waiman Long wrote:
>>> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
>>> index ca002c0..7a0ed1d 100644
>>> --- a/kernel/locking/lockdep.c
>>> +++ b/kernel/locking/lockdep.c
>>> @@ -139,6 +139,7 @@ static inline int debug_locks_off_graph_unlock(void)
>>> */
>>> unsigned long nr_lock_classes;
>>> static struct lock_class lock_classes[MAX_LOCKDEP_KEYS];
>>> +static DEFINE_PER_CPU(unsigned long [MAX_LOCKDEP_KEYS], lock_class_ops);
>>> @@ -1387,11 +1391,15 @@ static inline int usage_match(struct lock_list
>>> *entry, void *bit)
>>>
>>> static void print_lock_class_header(struct lock_class *class, int depth)
>>> {
>>> - int bit;
>>> + int bit, cpu;
>>> + unsigned long ops = 0UL;
>>> +
>>> + for_each_possible_cpu(cpu)
>>> + ops += *per_cpu(class->pops, cpu);
>>>
>>> printk("%*s->", depth, "");
>>> print_lock_name(class);
>>> - printk(KERN_CONT " ops: %lu", class->ops);
>>> + printk(KERN_CONT " ops: %lu", ops);
>>> printk(KERN_CONT " {\n");
>>>
>>> for (bit = 0; bit < LOCK_USAGE_STATES; bit++) {
>> That is an aweful lot of storage for a stupid number. Some archs
>> (sparc64) are bzImage size constrained and this will hurt them.
>>
>> Ingo, do you happen to remember what that number was good for?
> Just a spur of the moment statistics to satisfy curiousity, and it's useful
> to see how 'busy' a
> particular class is, right?
>
>> Can't we simply ditch it?
> We certainly could. Do we have roughly equivalent metrics to arrive at this
> number via other
> methods?
>
> Thanks,
>
> Ingo
One alternative is to group it under CONFIG_DEBUG_LOCKDEP again. This
metric was originally under CONFIG_DEBUG_LOCKDEP, but was moved to
CONFIG_LOCKDEP when trying to make other lock debugging statistics
per-cpu counters. It was probably because this metric is per lock class
while the rests are global.
By doing so, you incur the memory cost only when CONFIG_DEBUG_LOCKDEP is
defined.
What do you think?
Cheers,
Longman
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
On 10/02/2018 05:55 AM, Ingo Molnar wrote:
> * Peter Zijlstra wrote:
>
>> On Fri, Sep 28, 2018 at 01:53:20PM -0400, Waiman Long wrote:
>>> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
>>> index ca002c0..7a0ed1d 100644
>>> --- a/kernel/locking/lockdep.c
>>> +++ b/kernel/locking/lockdep.c
>>> @@ -139,6 +139,7 @@ static inline int debug_locks_off_graph_unlock(void)
>>> */
>>> unsigned long nr_lock_classes;
>>> static struct lock_class lock_classes[MAX_LOCKDEP_KEYS];
>>> +static DEFINE_PER_CPU(unsigned long [MAX_LOCKDEP_KEYS], lock_class_ops);
>>> @@ -1387,11 +1391,15 @@ static inline int usage_match(struct lock_list
>>> *entry, void *bit)
>>>
>>> static void print_lock_class_header(struct lock_class *class, int depth)
>>> {
>>> - int bit;
>>> + int bit, cpu;
>>> + unsigned long ops = 0UL;
>>> +
>>> + for_each_possible_cpu(cpu)
>>> + ops += *per_cpu(class->pops, cpu);
>>>
>>> printk("%*s->", depth, "");
>>> print_lock_name(class);
>>> - printk(KERN_CONT " ops: %lu", class->ops);
>>> + printk(KERN_CONT " ops: %lu", ops);
>>> printk(KERN_CONT " {\n");
>>>
>>> for (bit = 0; bit < LOCK_USAGE_STATES; bit++) {
>> That is an aweful lot of storage for a stupid number. Some archs
>> (sparc64) are bzImage size constrained and this will hurt them.
>>
>> Ingo, do you happen to remember what that number was good for?
> Just a spur of the moment statistics to satisfy curiousity, and it's useful
> to see how 'busy' a
> particular class is, right?
>
>> Can't we simply ditch it?
> We certainly could. Do we have roughly equivalent metrics to arrive at this
> number via other
> methods?
>
> Thanks,
>
> Ingo
One alternative is to group it under CONFIG_DEBUG_LOCKDEP again. This
metric was originally under CONFIG_DEBUG_LOCKDEP, but was moved to
CONFIG_LOCKDEP when trying to make other lock debugging statistics
per-cpu counters. It was probably because this metric is per lock class
while the rests are global.
By doing so, you incur the memory cost only when CONFIG_DEBUG_LOCKDEP is
defined.
What do you think?
Cheers,
Longman
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
* Peter Zijlstra wrote:
> On Fri, Sep 28, 2018 at 01:53:20PM -0400, Waiman Long wrote:
> > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> > index ca002c0..7a0ed1d 100644
> > --- a/kernel/locking/lockdep.c
> > +++ b/kernel/locking/lockdep.c
> > @@ -139,6 +139,7 @@ static inline int debug_locks_off_graph_unlock(void)
> > */
> > unsigned long nr_lock_classes;
> > static struct lock_class lock_classes[MAX_LOCKDEP_KEYS];
> > +static DEFINE_PER_CPU(unsigned long [MAX_LOCKDEP_KEYS], lock_class_ops);
>
> > @@ -1387,11 +1391,15 @@ static inline int usage_match(struct lock_list
> > *entry, void *bit)
> >
> > static void print_lock_class_header(struct lock_class *class, int depth)
> > {
> > - int bit;
> > + int bit, cpu;
> > + unsigned long ops = 0UL;
> > +
> > + for_each_possible_cpu(cpu)
> > + ops += *per_cpu(class->pops, cpu);
> >
> > printk("%*s->", depth, "");
> > print_lock_name(class);
> > - printk(KERN_CONT " ops: %lu", class->ops);
> > + printk(KERN_CONT " ops: %lu", ops);
> > printk(KERN_CONT " {\n");
> >
> > for (bit = 0; bit < LOCK_USAGE_STATES; bit++) {
>
> That is an aweful lot of storage for a stupid number. Some archs
> (sparc64) are bzImage size constrained and this will hurt them.
>
> Ingo, do you happen to remember what that number was good for?
Just a spur of the moment statistics to satisfy curiousity, and it's useful to
see how 'busy' a
particular class is, right?
> Can't we simply ditch it?
We certainly could. Do we have roughly equivalent metrics to arrive at this
number via other
methods?
Thanks,
Ingo
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
* Peter Zijlstra wrote:
> On Fri, Sep 28, 2018 at 01:53:20PM -0400, Waiman Long wrote:
> > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> > index ca002c0..7a0ed1d 100644
> > --- a/kernel/locking/lockdep.c
> > +++ b/kernel/locking/lockdep.c
> > @@ -139,6 +139,7 @@ static inline int debug_locks_off_graph_unlock(void)
> > */
> > unsigned long nr_lock_classes;
> > static struct lock_class lock_classes[MAX_LOCKDEP_KEYS];
> > +static DEFINE_PER_CPU(unsigned long [MAX_LOCKDEP_KEYS], lock_class_ops);
>
> > @@ -1387,11 +1391,15 @@ static inline int usage_match(struct lock_list
> > *entry, void *bit)
> >
> > static void print_lock_class_header(struct lock_class *class, int depth)
> > {
> > - int bit;
> > + int bit, cpu;
> > + unsigned long ops = 0UL;
> > +
> > + for_each_possible_cpu(cpu)
> > + ops += *per_cpu(class->pops, cpu);
> >
> > printk("%*s->", depth, "");
> > print_lock_name(class);
> > - printk(KERN_CONT " ops: %lu", class->ops);
> > + printk(KERN_CONT " ops: %lu", ops);
> > printk(KERN_CONT " {\n");
> >
> > for (bit = 0; bit < LOCK_USAGE_STATES; bit++) {
>
> That is an aweful lot of storage for a stupid number. Some archs
> (sparc64) are bzImage size constrained and this will hurt them.
>
> Ingo, do you happen to remember what that number was good for?
Just a spur of the moment statistics to satisfy curiousity, and it's useful to
see how 'busy' a
particular class is, right?
> Can't we simply ditch it?
We certainly could. Do we have roughly equivalent metrics to arrive at this
number via other
methods?
Thanks,
Ingo
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
On Fri, Sep 28, 2018 at 01:53:20PM -0400, Waiman Long wrote:
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index ca002c0..7a0ed1d 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -139,6 +139,7 @@ static inline int debug_locks_off_graph_unlock(void)
> */
> unsigned long nr_lock_classes;
> static struct lock_class lock_classes[MAX_LOCKDEP_KEYS];
> +static DEFINE_PER_CPU(unsigned long [MAX_LOCKDEP_KEYS], lock_class_ops);
> @@ -1387,11 +1391,15 @@ static inline int usage_match(struct lock_list
> *entry, void *bit)
>
> static void print_lock_class_header(struct lock_class *class, int depth)
> {
> - int bit;
> + int bit, cpu;
> + unsigned long ops = 0UL;
> +
> + for_each_possible_cpu(cpu)
> + ops += *per_cpu(class->pops, cpu);
>
> printk("%*s->", depth, "");
> print_lock_name(class);
> - printk(KERN_CONT " ops: %lu", class->ops);
> + printk(KERN_CONT " ops: %lu", ops);
> printk(KERN_CONT " {\n");
>
> for (bit = 0; bit < LOCK_USAGE_STATES; bit++) {
That is an aweful lot of storage for a stupid number. Some archs
(sparc64) are bzImage size constrained and this will hurt them.
Ingo, do you happen to remember what that number was good for?
Can't we simply ditch it?
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
On Fri, Sep 28, 2018 at 01:53:20PM -0400, Waiman Long wrote:
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index ca002c0..7a0ed1d 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -139,6 +139,7 @@ static inline int debug_locks_off_graph_unlock(void)
> */
> unsigned long nr_lock_classes;
> static struct lock_class lock_classes[MAX_LOCKDEP_KEYS];
> +static DEFINE_PER_CPU(unsigned long [MAX_LOCKDEP_KEYS], lock_class_ops);
> @@ -1387,11 +1391,15 @@ static inline int usage_match(struct lock_list
> *entry, void *bit)
>
> static void print_lock_class_header(struct lock_class *class, int depth)
> {
> - int bit;
> + int bit, cpu;
> + unsigned long ops = 0UL;
> +
> + for_each_possible_cpu(cpu)
> + ops += *per_cpu(class->pops, cpu);
>
> printk("%*s->", depth, "");
> print_lock_name(class);
> - printk(KERN_CONT " ops: %lu", class->ops);
> + printk(KERN_CONT " ops: %lu", ops);
> printk(KERN_CONT " {\n");
>
> for (bit = 0; bit < LOCK_USAGE_STATES; bit++) {
That is an aweful lot of storage for a stupid number. Some archs
(sparc64) are bzImage size constrained and this will hurt them.
Ingo, do you happen to remember what that number was good for?
Can't we simply ditch it?
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
Hi Waiman,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tip/locking/core]
[also build test ERROR on v4.19-rc5 next-20180928]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Waiman-Long/locking-lockdep-Remove-add_chain_cache_classes/20180929-031820
config: x86_64-randconfig-u0-09290404 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
kernel/locking/lockdep_proc.c: In function 'l_show':
>> kernel/locking/lockdep_proc.c:71:34: error: 'struct lock_class' has no
>> member named 'ops'
seq_printf(m, " OPS:%8ld", class->ops);
^
vim +71 kernel/locking/lockdep_proc.c
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 57
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 58 static int
l_show(struct seq_file *m, void *v)
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 59 {
8109e1de8 kernel/lockdep_proc.c Li Zefan 2009-08-17 60struct
lock_class *class = list_entry(v, struct lock_class, lock_entry);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 61struct
lock_list *entry;
f510b233c kernel/lockdep_proc.c Peter Zijlstra2009-01-22 62char
usage[LOCK_USAGE_CHARS];
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 63
8109e1de8 kernel/lockdep_proc.c Li Zefan 2009-08-17 64if (v
== _lock_classes) {
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 65
seq_printf(m, "all lock classes:\n");
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 66
return 0;
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 67}
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 68
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 69
seq_printf(m, "%p", class->key);
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 70 #ifdef
CONFIG_DEBUG_LOCKDEP
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 @71
seq_printf(m, " OPS:%8ld", class->ops);
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 72 #endif
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 73 #ifdef
CONFIG_PROVE_LOCKING
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 74
seq_printf(m, " FD:%5ld", lockdep_count_forward_deps(class));
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 75
seq_printf(m, " BD:%5ld", lockdep_count_backward_deps(class));
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 76 #endif
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 77
f510b233c kernel/lockdep_proc.c Peter Zijlstra2009-01-22 78
get_usage_chars(class, usage);
f510b233c kernel/lockdep_proc.c Peter Zijlstra2009-01-22 79
seq_printf(m, " %s", usage);
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 80
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 81
seq_printf(m, ": ");
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 82
print_name(m, class);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 83
seq_puts(m, "\n");
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 84
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 85
list_for_each_entry(entry, >locks_after, entry) {
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 86
if (entry->distance == 1) {
2429e4ee7 kernel/lockdep_proc.c Huang, Ying 2008-06-13 87
seq_printf(m, " -> [%p] ", entry->class->key);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 88
print_name(m, entry->class);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 89
seq_puts(m, "\n");
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 90
}
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 91}
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 92
seq_puts(m, "\n");
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 93
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 94return
0;
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 95 }
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 96
:: The code at line 71 was first introduced by commit
:: a8f24a3978c5f82419e1c90dc90460731204f46f [PATCH] lockdep: procfs
:: TO: Ingo Molnar
:: CC: Linus Torvalds
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
Hi Waiman,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tip/locking/core]
[also build test ERROR on v4.19-rc5 next-20180928]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Waiman-Long/locking-lockdep-Remove-add_chain_cache_classes/20180929-031820
config: x86_64-randconfig-u0-09290404 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
kernel/locking/lockdep_proc.c: In function 'l_show':
>> kernel/locking/lockdep_proc.c:71:34: error: 'struct lock_class' has no
>> member named 'ops'
seq_printf(m, " OPS:%8ld", class->ops);
^
vim +71 kernel/locking/lockdep_proc.c
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 57
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 58 static int
l_show(struct seq_file *m, void *v)
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 59 {
8109e1de8 kernel/lockdep_proc.c Li Zefan 2009-08-17 60struct
lock_class *class = list_entry(v, struct lock_class, lock_entry);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 61struct
lock_list *entry;
f510b233c kernel/lockdep_proc.c Peter Zijlstra2009-01-22 62char
usage[LOCK_USAGE_CHARS];
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 63
8109e1de8 kernel/lockdep_proc.c Li Zefan 2009-08-17 64if (v
== _lock_classes) {
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 65
seq_printf(m, "all lock classes:\n");
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 66
return 0;
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 67}
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 68
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 69
seq_printf(m, "%p", class->key);
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 70 #ifdef
CONFIG_DEBUG_LOCKDEP
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 @71
seq_printf(m, " OPS:%8ld", class->ops);
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 72 #endif
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 73 #ifdef
CONFIG_PROVE_LOCKING
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 74
seq_printf(m, " FD:%5ld", lockdep_count_forward_deps(class));
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 75
seq_printf(m, " BD:%5ld", lockdep_count_backward_deps(class));
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 76 #endif
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 77
f510b233c kernel/lockdep_proc.c Peter Zijlstra2009-01-22 78
get_usage_chars(class, usage);
f510b233c kernel/lockdep_proc.c Peter Zijlstra2009-01-22 79
seq_printf(m, " %s", usage);
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 80
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 81
seq_printf(m, ": ");
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 82
print_name(m, class);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 83
seq_puts(m, "\n");
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 84
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 85
list_for_each_entry(entry, >locks_after, entry) {
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 86
if (entry->distance == 1) {
2429e4ee7 kernel/lockdep_proc.c Huang, Ying 2008-06-13 87
seq_printf(m, " -> [%p] ", entry->class->key);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 88
print_name(m, entry->class);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 89
seq_puts(m, "\n");
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 90
}
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 91}
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 92
seq_puts(m, "\n");
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 93
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 94return
0;
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 95 }
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 96
:: The code at line 71 was first introduced by commit
:: a8f24a3978c5f82419e1c90dc90460731204f46f [PATCH] lockdep: procfs
:: TO: Ingo Molnar
:: CC: Linus Torvalds
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
On 09/28/2018 04:25 PM, kbuild test robot wrote: > Hi Waiman, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on tip/locking/core] > [also build test ERROR on v4.19-rc5 next-20180928] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Waiman-Long/locking-lockdep-Remove-add_chain_cache_classes/20180929-031820 > config: i386-randconfig-s2-201838 (attached as .config) > compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026 > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 > > All errors (new ones prefixed by >>): > >kernel/locking/lockdep_proc.c: In function 'l_show': >>> kernel/locking/lockdep_proc.c:71:34: error: 'struct lock_class' has no >>> member named 'ops'; did you mean 'pops'? > seq_printf(m, " OPS:%8ld", class->ops); > ^~ OK, I missed the ops reference in lockdep_proc.c which is probably not built in my debug kernel config. I will fix that in the next version. -Longman
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
On 09/28/2018 04:25 PM, kbuild test robot wrote: > Hi Waiman, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on tip/locking/core] > [also build test ERROR on v4.19-rc5 next-20180928] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Waiman-Long/locking-lockdep-Remove-add_chain_cache_classes/20180929-031820 > config: i386-randconfig-s2-201838 (attached as .config) > compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026 > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 > > All errors (new ones prefixed by >>): > >kernel/locking/lockdep_proc.c: In function 'l_show': >>> kernel/locking/lockdep_proc.c:71:34: error: 'struct lock_class' has no >>> member named 'ops'; did you mean 'pops'? > seq_printf(m, " OPS:%8ld", class->ops); > ^~ OK, I missed the ops reference in lockdep_proc.c which is probably not built in my debug kernel config. I will fix that in the next version. -Longman
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
Hi Waiman,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tip/locking/core]
[also build test ERROR on v4.19-rc5 next-20180928]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Waiman-Long/locking-lockdep-Remove-add_chain_cache_classes/20180929-031820
config: i386-randconfig-s2-201838 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
kernel/locking/lockdep_proc.c: In function 'l_show':
>> kernel/locking/lockdep_proc.c:71:34: error: 'struct lock_class' has no
>> member named 'ops'; did you mean 'pops'?
seq_printf(m, " OPS:%8ld", class->ops);
^~
vim +71 kernel/locking/lockdep_proc.c
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 57
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 58 static int
l_show(struct seq_file *m, void *v)
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 59 {
8109e1de8 kernel/lockdep_proc.c Li Zefan 2009-08-17 60struct
lock_class *class = list_entry(v, struct lock_class, lock_entry);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 61struct
lock_list *entry;
f510b233c kernel/lockdep_proc.c Peter Zijlstra2009-01-22 62char
usage[LOCK_USAGE_CHARS];
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 63
8109e1de8 kernel/lockdep_proc.c Li Zefan 2009-08-17 64if (v
== _lock_classes) {
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 65
seq_printf(m, "all lock classes:\n");
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 66
return 0;
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 67}
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 68
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 69
seq_printf(m, "%p", class->key);
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 70 #ifdef
CONFIG_DEBUG_LOCKDEP
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 @71
seq_printf(m, " OPS:%8ld", class->ops);
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 72 #endif
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 73 #ifdef
CONFIG_PROVE_LOCKING
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 74
seq_printf(m, " FD:%5ld", lockdep_count_forward_deps(class));
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 75
seq_printf(m, " BD:%5ld", lockdep_count_backward_deps(class));
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 76 #endif
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 77
f510b233c kernel/lockdep_proc.c Peter Zijlstra2009-01-22 78
get_usage_chars(class, usage);
f510b233c kernel/lockdep_proc.c Peter Zijlstra2009-01-22 79
seq_printf(m, " %s", usage);
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 80
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 81
seq_printf(m, ": ");
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 82
print_name(m, class);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 83
seq_puts(m, "\n");
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 84
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 85
list_for_each_entry(entry, >locks_after, entry) {
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 86
if (entry->distance == 1) {
2429e4ee7 kernel/lockdep_proc.c Huang, Ying 2008-06-13 87
seq_printf(m, " -> [%p] ", entry->class->key);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 88
print_name(m, entry->class);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 89
seq_puts(m, "\n");
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 90
}
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 91}
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 92
seq_puts(m, "\n");
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 93
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 94return
0;
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 95 }
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 96
:: The code at line 71 was first introduced by commit
:: a8f24a3978c5f82419e1c90dc90460731204f46f [PATCH] lockdep: procfs
:: TO: Ingo Molnar
:: CC: Linus Torvalds
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all
Re: [PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
Hi Waiman,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tip/locking/core]
[also build test ERROR on v4.19-rc5 next-20180928]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Waiman-Long/locking-lockdep-Remove-add_chain_cache_classes/20180929-031820
config: i386-randconfig-s2-201838 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
kernel/locking/lockdep_proc.c: In function 'l_show':
>> kernel/locking/lockdep_proc.c:71:34: error: 'struct lock_class' has no
>> member named 'ops'; did you mean 'pops'?
seq_printf(m, " OPS:%8ld", class->ops);
^~
vim +71 kernel/locking/lockdep_proc.c
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 57
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 58 static int
l_show(struct seq_file *m, void *v)
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 59 {
8109e1de8 kernel/lockdep_proc.c Li Zefan 2009-08-17 60struct
lock_class *class = list_entry(v, struct lock_class, lock_entry);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 61struct
lock_list *entry;
f510b233c kernel/lockdep_proc.c Peter Zijlstra2009-01-22 62char
usage[LOCK_USAGE_CHARS];
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 63
8109e1de8 kernel/lockdep_proc.c Li Zefan 2009-08-17 64if (v
== _lock_classes) {
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 65
seq_printf(m, "all lock classes:\n");
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 66
return 0;
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 67}
94c61c0ae kernel/lockdep_proc.c Tim Pepper2007-10-11 68
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 69
seq_printf(m, "%p", class->key);
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 70 #ifdef
CONFIG_DEBUG_LOCKDEP
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 @71
seq_printf(m, " OPS:%8ld", class->ops);
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 72 #endif
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 73 #ifdef
CONFIG_PROVE_LOCKING
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 74
seq_printf(m, " FD:%5ld", lockdep_count_forward_deps(class));
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 75
seq_printf(m, " BD:%5ld", lockdep_count_backward_deps(class));
df60a8441 kernel/lockdep_proc.c Stephen Hemminger 2008-08-15 76 #endif
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 77
f510b233c kernel/lockdep_proc.c Peter Zijlstra2009-01-22 78
get_usage_chars(class, usage);
f510b233c kernel/lockdep_proc.c Peter Zijlstra2009-01-22 79
seq_printf(m, " %s", usage);
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 80
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 81
seq_printf(m, ": ");
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 82
print_name(m, class);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 83
seq_puts(m, "\n");
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 84
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 85
list_for_each_entry(entry, >locks_after, entry) {
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 86
if (entry->distance == 1) {
2429e4ee7 kernel/lockdep_proc.c Huang, Ying 2008-06-13 87
seq_printf(m, " -> [%p] ", entry->class->key);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 88
print_name(m, entry->class);
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 89
seq_puts(m, "\n");
068135e63 kernel/lockdep_proc.c Jason Baron 2007-02-10 90
}
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 91}
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 92
seq_puts(m, "\n");
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 93
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 94return
0;
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 95 }
a8f24a397 kernel/lockdep_proc.c Ingo Molnar 2006-07-03 96
:: The code at line 71 was first introduced by commit
:: a8f24a3978c5f82419e1c90dc90460731204f46f [PATCH] lockdep: procfs
:: TO: Ingo Molnar
:: CC: Linus Torvalds
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all
[PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
A sizable portion of the CPU cycles spent on the __lock_acquire() is used
up by the atomic increment of class->ops stat counter. By changing it
to a per-cpu counter, we can reduce the amount of cacheline contention
on the class structure when multiple CPUs are trying to acquire locks
of the same class simultaneously.
This patch also fixes a bug in the increment code as the counter is of
the unsigned long type, but atomic_inc() was used to increment it.
Signed-off-by: Waiman Long
---
include/linux/lockdep.h | 2 +-
kernel/locking/lockdep.c | 18 ++
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index b0d0b51..f8bf705 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -102,7 +102,7 @@ struct lock_class {
/*
* Statistics counter:
*/
- unsigned long ops;
+ unsigned long __percpu *pops;
const char *name;
int name_version;
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index ca002c0..7a0ed1d 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -139,6 +139,7 @@ static inline int debug_locks_off_graph_unlock(void)
*/
unsigned long nr_lock_classes;
static struct lock_class lock_classes[MAX_LOCKDEP_KEYS];
+static DEFINE_PER_CPU(unsigned long [MAX_LOCKDEP_KEYS], lock_class_ops);
static inline struct lock_class *hlock_class(struct held_lock *hlock)
{
@@ -784,11 +785,14 @@ static bool assign_lock_key(struct lockdep_map *lock)
dump_stack();
return NULL;
}
- class = lock_classes + nr_lock_classes++;
+ class = lock_classes + nr_lock_classes;
debug_atomic_inc(nr_unused_locks);
class->key = key;
class->name = lock->name;
class->subclass = subclass;
+ class->pops = _class_ops[nr_lock_classes];
+ nr_lock_classes++;
+
INIT_LIST_HEAD(>lock_entry);
INIT_LIST_HEAD(>locks_before);
INIT_LIST_HEAD(>locks_after);
@@ -1387,11 +1391,15 @@ static inline int usage_match(struct lock_list *entry,
void *bit)
static void print_lock_class_header(struct lock_class *class, int depth)
{
- int bit;
+ int bit, cpu;
+ unsigned long ops = 0UL;
+
+ for_each_possible_cpu(cpu)
+ ops += *per_cpu(class->pops, cpu);
printk("%*s->", depth, "");
print_lock_name(class);
- printk(KERN_CONT " ops: %lu", class->ops);
+ printk(KERN_CONT " ops: %lu", ops);
printk(KERN_CONT " {\n");
for (bit = 0; bit < LOCK_USAGE_STATES; bit++) {
@@ -3226,7 +3234,9 @@ static int __lock_acquire(struct lockdep_map *lock,
unsigned int subclass,
if (!class)
return 0;
}
- atomic_inc((atomic_t *)>ops);
+
+ __this_cpu_inc(*class->pops);
+
if (very_verbose(class)) {
printk("\nacquire class [%px] %s", class->key, class->name);
if (class->name_version > 1)
--
1.8.3.1
[PATCH 4/5] locking/lockdep: Make class->ops a percpu counter
A sizable portion of the CPU cycles spent on the __lock_acquire() is used
up by the atomic increment of class->ops stat counter. By changing it
to a per-cpu counter, we can reduce the amount of cacheline contention
on the class structure when multiple CPUs are trying to acquire locks
of the same class simultaneously.
This patch also fixes a bug in the increment code as the counter is of
the unsigned long type, but atomic_inc() was used to increment it.
Signed-off-by: Waiman Long
---
include/linux/lockdep.h | 2 +-
kernel/locking/lockdep.c | 18 ++
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index b0d0b51..f8bf705 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -102,7 +102,7 @@ struct lock_class {
/*
* Statistics counter:
*/
- unsigned long ops;
+ unsigned long __percpu *pops;
const char *name;
int name_version;
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index ca002c0..7a0ed1d 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -139,6 +139,7 @@ static inline int debug_locks_off_graph_unlock(void)
*/
unsigned long nr_lock_classes;
static struct lock_class lock_classes[MAX_LOCKDEP_KEYS];
+static DEFINE_PER_CPU(unsigned long [MAX_LOCKDEP_KEYS], lock_class_ops);
static inline struct lock_class *hlock_class(struct held_lock *hlock)
{
@@ -784,11 +785,14 @@ static bool assign_lock_key(struct lockdep_map *lock)
dump_stack();
return NULL;
}
- class = lock_classes + nr_lock_classes++;
+ class = lock_classes + nr_lock_classes;
debug_atomic_inc(nr_unused_locks);
class->key = key;
class->name = lock->name;
class->subclass = subclass;
+ class->pops = _class_ops[nr_lock_classes];
+ nr_lock_classes++;
+
INIT_LIST_HEAD(>lock_entry);
INIT_LIST_HEAD(>locks_before);
INIT_LIST_HEAD(>locks_after);
@@ -1387,11 +1391,15 @@ static inline int usage_match(struct lock_list *entry,
void *bit)
static void print_lock_class_header(struct lock_class *class, int depth)
{
- int bit;
+ int bit, cpu;
+ unsigned long ops = 0UL;
+
+ for_each_possible_cpu(cpu)
+ ops += *per_cpu(class->pops, cpu);
printk("%*s->", depth, "");
print_lock_name(class);
- printk(KERN_CONT " ops: %lu", class->ops);
+ printk(KERN_CONT " ops: %lu", ops);
printk(KERN_CONT " {\n");
for (bit = 0; bit < LOCK_USAGE_STATES; bit++) {
@@ -3226,7 +3234,9 @@ static int __lock_acquire(struct lockdep_map *lock,
unsigned int subclass,
if (!class)
return 0;
}
- atomic_inc((atomic_t *)>ops);
+
+ __this_cpu_inc(*class->pops);
+
if (very_verbose(class)) {
printk("\nacquire class [%px] %s", class->key, class->name);
if (class->name_version > 1)
--
1.8.3.1
