2009-09-22 06:10:30

by coder

[permalink] [raw]
Subject: [PATCH]lock_stat: Adding "nr_contender" and "max_contender" to show the degree of contention on ticket spinlock

The cost of spin lock operations is increased as the number of threads
who wait for the spinlock increased. We are able to record the number
of threads which includes the holder and waiters for the ticket
spinlock when the contention happened (The spinlock is not free when a
thread wants to lock it). So, we introduces two parameters to
lock_stat: "nr_contender" and "max_contender".
nr_contender is the sum of the number of holder and waiters when
contention happened. So, "nr_contender / contentions" represent the
average degree of contention.
Max_contender is the biggest number of holder and waiters when
contention happened. So, max_contender represents the highest degree
of contention.

Signed-off-by: Li Zefan <[email protected]>
---
arch/x86/include/asm/spinlock.h | 9 +++++++++
include/linux/lockdep.h | 2 ++
include/linux/spinlock.h | 4 ++++
kernel/lockdep.c | 21 ++++++++++++++++++++-
kernel/lockdep_proc.c | 8 ++++++--
5 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/spinlock.h b/arch/x86/include/asm/spinlock.h
index 4e77853..a2396d8 100644
--- a/arch/x86/include/asm/spinlock.h
+++ b/arch/x86/include/asm/spinlock.h
@@ -172,6 +172,15 @@ static inline int
__ticket_spin_is_contended(raw_spinlock_t *lock)
return (((tmp >> TICKET_SHIFT) - tmp) & ((1 << TICKET_SHIFT) - 1)) > 1;
}

+static inline int __ticket_spin_nr_contender(raw_spinlock_t *lock)
+{
+ int tmp = ACCESS_ONCE(lock->slock);
+
+ return (((tmp >> TICKET_SHIFT) - tmp) & ((1 << TICKET_SHIFT) - 1)) + 1;
+}
+
+#define spin_nr_contender(lock) __ticket_spin_nr_contender(&(lock)->raw_lock)
+
#ifndef CONFIG_PARAVIRT_SPINLOCKS

static inline int __raw_spin_is_locked(raw_spinlock_t *lock)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 9ccf0e2..0627706 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -106,6 +106,8 @@ enum bounce_type {
bounce_acquired_read,
bounce_contended_write,
bounce_contended_read,
+ nr_contender,
+ max_contender,
nr_bounce_types,

bounce_acquired = bounce_acquired_write,
diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index f0ca7a7..3d86966 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -132,6 +132,10 @@ do { \
#endif /*__raw_spin_is_contended*/
#endif

+#ifndef spin_nr_contender
+#define spin_nr_contender(lock) (spin_is_contended(lock) ? 1 : 0)
+#endif
+
/* The lock does not imply full memory barrier. */
#ifndef ARCH_HAS_SMP_MB_AFTER_LOCK
static inline void smp_mb__after_lock(void) { smp_mb(); }
diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index f74d2d7..e4b9f80 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -200,8 +200,14 @@ struct lock_class_stats lock_stats(struct
lock_class *class)
lock_time_add(&pcs->read_holdtime, &stats.read_holdtime);
lock_time_add(&pcs->write_holdtime, &stats.write_holdtime);

- for (i = 0; i < ARRAY_SIZE(stats.bounces); i++)
+ for (i = 0; i < ARRAY_SIZE(stats.bounces); i++) {
+ if (i == max_contender) {
+ if (stats.bounces[i] < pcs->bounces[i])
+ stats.bounces[i] = pcs->bounces[i];
+ continue;
+ }
stats.bounces[i] += pcs->bounces[i];
+ }
}

return stats;
@@ -3293,9 +3299,15 @@ __lock_contended(struct lockdep_map *lock,
unsigned long ip)
struct task_struct *curr = current;
struct held_lock *hlock, *prev_hlock;
struct lock_class_stats *stats;
+ unsigned long contender;
+ spinlock_t *lock_ptr;
unsigned int depth;
int i, contention_point, contending_point;

+ lock_ptr = container_of(lock, spinlock_t, dep_map);
+ if (lock_ptr->magic == SPINLOCK_MAGIC)
+ contender = spin_nr_contender(lock_ptr);
+
depth = curr->lockdep_depth;
if (DEBUG_LOCKS_WARN_ON(!depth))
return;
@@ -3332,6 +3344,13 @@ found_it:
stats->contending_point[contending_point]++;
if (lock->cpu != smp_processor_id())
stats->bounces[bounce_contended + !!hlock->read]++;
+
+ if (lock_ptr->magic == SPINLOCK_MAGIC) {
+ stats->bounces[nr_contender] += contender;
+ if (stats->bounces[max_contender] < contender)
+ stats->bounces[max_contender] = contender;
+ }
+
put_lock_stats(stats);
}

diff --git a/kernel/lockdep_proc.c b/kernel/lockdep_proc.c
index d4b3dbc..a3d0910 100644
--- a/kernel/lockdep_proc.c
+++ b/kernel/lockdep_proc.c
@@ -472,6 +472,8 @@ static void seq_stats(struct seq_file *m, struct
lock_stat_data *data)
seq_printf(m, "%40s:", name);

seq_printf(m, "%14lu ", stats->bounces[bounce_contended_write]);
+ seq_printf(m, "%14lu ", stats->bounces[nr_contender]);
+ seq_printf(m, "%14lu ", stats->bounces[max_contender]);
seq_lock_time(m, &stats->write_waittime);
seq_printf(m, " %14lu ", stats->bounces[bounce_acquired_write]);
seq_lock_time(m, &stats->write_holdtime);
@@ -541,11 +543,13 @@ static void seq_header(struct seq_file *m)
if (unlikely(!debug_locks))
seq_printf(m, "*WARNING* lock debugging disabled!! - possibly due
to a lockdep warning\n");

- seq_line(m, '-', 0, 40 + 1 + 10 * (14 + 1));
- seq_printf(m, "%40s %14s %14s %14s %14s %14s %14s %14s %14s "
+ seq_line(m, '-', 0, 40 + 1 + 12 * (14 + 1));
+ seq_printf(m, "%40s %14s %14s %14s %14s %14s %14s %14s %14s %14s %14s "
"%14s %14s\n",
"class name",
"con-bounces",
+ "nr-contender",
+ "contender-max",
"contentions",
"waittime-min",
"waittime-max",


2009-09-22 06:21:22

by coder

[permalink] [raw]
Subject: Re: [PATCH]lock_stat: Adding "nr_contender" and "max_contender" to show the degree of contention on ticket spinlock

Sorry, make wrong Signed-off-by.
Should be
Signed-off-by: Xi Yang <[email protected]>

On Tue, Sep 22, 2009 at 4:10 PM, Yang Xi <[email protected]> wrote:
> The cost of spin lock operations is increased as the number of threads
> who wait for the spinlock increased. We are able to record the number
> of threads which includes the holder and waiters for the ticket
> spinlock when the contention happened (The spinlock is not free when a
> thread wants to lock it). So, we introduces two parameters to
> lock_stat: "nr_contender" and "max_contender".
> nr_contender is the sum of the number of holder and waiters when
> contention happened. So, "nr_contender / contentions" represent the
> average degree of contention.
> Max_contender is the biggest number of holder and waiters when
> contention happened. So, max_contender represents the highest degree
> of contention.
>
> Signed-off-by: Li Zefan <[email protected]>
> ---
> ?arch/x86/include/asm/spinlock.h | ? ?9 +++++++++
> ?include/linux/lockdep.h ? ? ? ? | ? ?2 ++
> ?include/linux/spinlock.h ? ? ? ?| ? ?4 ++++
> ?kernel/lockdep.c ? ? ? ? ? ? ? ?| ? 21 ++++++++++++++++++++-
> ?kernel/lockdep_proc.c ? ? ? ? ? | ? ?8 ++++++--
> ?5 files changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/spinlock.h b/arch/x86/include/asm/spinlock.h
> index 4e77853..a2396d8 100644
> --- a/arch/x86/include/asm/spinlock.h
> +++ b/arch/x86/include/asm/spinlock.h
> @@ -172,6 +172,15 @@ static inline int
> __ticket_spin_is_contended(raw_spinlock_t *lock)
> ? ? ? ?return (((tmp >> TICKET_SHIFT) - tmp) & ((1 << TICKET_SHIFT) - 1)) > 1;
> ?}
>
> +static inline int __ticket_spin_nr_contender(raw_spinlock_t *lock)
> +{
> + ? ? ? int tmp = ACCESS_ONCE(lock->slock);
> +
> + ? ? ? return (((tmp >> TICKET_SHIFT) - tmp) & ((1 << TICKET_SHIFT) - 1)) + 1;
> +}
> +
> +#define spin_nr_contender(lock) __ticket_spin_nr_contender(&(lock)->raw_lock)
> +
> ?#ifndef CONFIG_PARAVIRT_SPINLOCKS
>
> ?static inline int __raw_spin_is_locked(raw_spinlock_t *lock)
> diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> index 9ccf0e2..0627706 100644
> --- a/include/linux/lockdep.h
> +++ b/include/linux/lockdep.h
> @@ -106,6 +106,8 @@ enum bounce_type {
> ? ? ? ?bounce_acquired_read,
> ? ? ? ?bounce_contended_write,
> ? ? ? ?bounce_contended_read,
> + ? ? ? nr_contender,
> + ? ? ? max_contender,
> ? ? ? ?nr_bounce_types,
>
> ? ? ? ?bounce_acquired = bounce_acquired_write,
> diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
> index f0ca7a7..3d86966 100644
> --- a/include/linux/spinlock.h
> +++ b/include/linux/spinlock.h
> @@ -132,6 +132,10 @@ do { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ?#endif /*__raw_spin_is_contended*/
> ?#endif
>
> +#ifndef spin_nr_contender
> +#define spin_nr_contender(lock) (spin_is_contended(lock) ? 1 : 0)
> +#endif
> +
> ?/* The lock does not imply full memory barrier. */
> ?#ifndef ARCH_HAS_SMP_MB_AFTER_LOCK
> ?static inline void smp_mb__after_lock(void) { smp_mb(); }
> diff --git a/kernel/lockdep.c b/kernel/lockdep.c
> index f74d2d7..e4b9f80 100644
> --- a/kernel/lockdep.c
> +++ b/kernel/lockdep.c
> @@ -200,8 +200,14 @@ struct lock_class_stats lock_stats(struct
> lock_class *class)
> ? ? ? ? ? ? ? ?lock_time_add(&pcs->read_holdtime, &stats.read_holdtime);
> ? ? ? ? ? ? ? ?lock_time_add(&pcs->write_holdtime, &stats.write_holdtime);
>
> - ? ? ? ? ? ? ? for (i = 0; i < ARRAY_SIZE(stats.bounces); i++)
> + ? ? ? ? ? ? ? for (i = 0; i < ARRAY_SIZE(stats.bounces); i++) {
> + ? ? ? ? ? ? ? ? ? ? ? if (i == max_contender) {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? if (stats.bounces[i] < pcs->bounces[i])
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? stats.bounces[i] = pcs->bounces[i];
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? continue;
> + ? ? ? ? ? ? ? ? ? ? ? }
> ? ? ? ? ? ? ? ? ? ? ? ?stats.bounces[i] += pcs->bounces[i];
> + ? ? ? ? ? ? ? }
> ? ? ? ?}
>
> ? ? ? ?return stats;
> @@ -3293,9 +3299,15 @@ __lock_contended(struct lockdep_map *lock,
> unsigned long ip)
> ? ? ? ?struct task_struct *curr = current;
> ? ? ? ?struct held_lock *hlock, *prev_hlock;
> ? ? ? ?struct lock_class_stats *stats;
> + ? ? ? unsigned long contender;
> + ? ? ? spinlock_t *lock_ptr;
> ? ? ? ?unsigned int depth;
> ? ? ? ?int i, contention_point, contending_point;
>
> + ? ? ? lock_ptr = container_of(lock, spinlock_t, dep_map);
> + ? ? ? if (lock_ptr->magic == SPINLOCK_MAGIC)
> + ? ? ? ? ? ? ? contender = spin_nr_contender(lock_ptr);
> +
> ? ? ? ?depth = curr->lockdep_depth;
> ? ? ? ?if (DEBUG_LOCKS_WARN_ON(!depth))
> ? ? ? ? ? ? ? ?return;
> @@ -3332,6 +3344,13 @@ found_it:
> ? ? ? ? ? ? ? ?stats->contending_point[contending_point]++;
> ? ? ? ?if (lock->cpu != smp_processor_id())
> ? ? ? ? ? ? ? ?stats->bounces[bounce_contended + !!hlock->read]++;
> +
> + ? ? ? if (lock_ptr->magic == SPINLOCK_MAGIC) {
> + ? ? ? ? ? ? ? stats->bounces[nr_contender] += contender;
> + ? ? ? ? ? ? ? if (stats->bounces[max_contender] < contender)
> + ? ? ? ? ? ? ? ? ? ? ? stats->bounces[max_contender] = contender;
> + ? ? ? }
> +
> ? ? ? ?put_lock_stats(stats);
> ?}
>
> diff --git a/kernel/lockdep_proc.c b/kernel/lockdep_proc.c
> index d4b3dbc..a3d0910 100644
> --- a/kernel/lockdep_proc.c
> +++ b/kernel/lockdep_proc.c
> @@ -472,6 +472,8 @@ static void seq_stats(struct seq_file *m, struct
> lock_stat_data *data)
> ? ? ? ? ? ? ? ? ? ? ? ?seq_printf(m, "%40s:", name);
>
> ? ? ? ? ? ? ? ?seq_printf(m, "%14lu ", stats->bounces[bounce_contended_write]);
> + ? ? ? ? ? ? ? seq_printf(m, "%14lu ", stats->bounces[nr_contender]);
> + ? ? ? ? ? ? ? seq_printf(m, "%14lu ", stats->bounces[max_contender]);
> ? ? ? ? ? ? ? ?seq_lock_time(m, &stats->write_waittime);
> ? ? ? ? ? ? ? ?seq_printf(m, " %14lu ", stats->bounces[bounce_acquired_write]);
> ? ? ? ? ? ? ? ?seq_lock_time(m, &stats->write_holdtime);
> @@ -541,11 +543,13 @@ static void seq_header(struct seq_file *m)
> ? ? ? ?if (unlikely(!debug_locks))
> ? ? ? ? ? ? ? ?seq_printf(m, "*WARNING* lock debugging disabled!! - possibly due
> to a lockdep warning\n");
>
> - ? ? ? seq_line(m, '-', 0, 40 + 1 + 10 * (14 + 1));
> - ? ? ? seq_printf(m, "%40s %14s %14s %14s %14s %14s %14s %14s %14s "
> + ? ? ? seq_line(m, '-', 0, 40 + 1 + 12 * (14 + 1));
> + ? ? ? seq_printf(m, "%40s %14s %14s %14s %14s %14s %14s %14s %14s %14s %14s "
> ? ? ? ? ? ? ? ? ? ? ? ?"%14s %14s\n",
> ? ? ? ? ? ? ? ? ? ? ? ?"class name",
> ? ? ? ? ? ? ? ? ? ? ? ?"con-bounces",
> + ? ? ? ? ? ? ? ? ? ? ? "nr-contender",
> + ? ? ? ? ? ? ? ? ? ? ? "contender-max",
> ? ? ? ? ? ? ? ? ? ? ? ?"contentions",
> ? ? ? ? ? ? ? ? ? ? ? ?"waittime-min",
> ? ? ? ? ? ? ? ? ? ? ? ?"waittime-max",
>

2009-09-22 13:42:13

by Daniel Walker

[permalink] [raw]
Subject: Re: [PATCH]lock_stat: Adding "nr_contender" and "max_contender" to show the degree of contention on ticket spinlock

On Tue, 2009-09-22 at 16:10 +1000, Yang Xi wrote:
> diff --git a/arch/x86/include/asm/spinlock.h b/arch/x86/include/asm/spinlock.h
> index 4e77853..a2396d8 100644
> --- a/arch/x86/include/asm/spinlock.h
> +++ b/arch/x86/include/asm/spinlock.h
> @@ -172,6 +172,15 @@ static inline int
> __ticket_spin_is_contended(raw_spinlock_t *lock)
> return (((tmp >> TICKET_SHIFT) - tmp) & ((1 << TICKET_SHIFT) - 1)) > 1;
> }


You patch is line wrapped .. It means it might not be easy to apply it..
You need to tell your mailer that the patch is "preformatted" . In the
kernel tree look at "Documentation/email-clients.txt" for more
information.

(btw, since your using git anyway you might want to look into using
git-format-patch and git-send-email for submitting)

Daniel