* [Xenomai-core] [PATCH 1/3] decouple spinlock stats from XENO_OPT_STATS
@ 2006-11-20 23:14 Jan Kiszka
2006-11-21 16:40 ` Gilles Chanteperdrix
2006-11-22 7:20 ` Jan Kiszka
0 siblings, 2 replies; 7+ messages in thread
From: Jan Kiszka @ 2006-11-20 23:14 UTC (permalink / raw)
To: xenomai-core
[-- Attachment #1.1: Type: text/plain, Size: 441 bytes --]
As recently discussed, the current implicit coupling of spinlock
statistics/debugging with the XENO_OPT_STATS switch is suboptimal. The
reason: on SMP, it adds noticeable overhead to the STATS feature which
may only be used for thread statistics collection (as also advertised in
the kconfig help).
This patch makes the spinlock debugging and lock length collection
feature only depend on XENO_OPT_DEBUG (as a first step).
Jan
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: decouple-spinlock-dbg-from-stats.patch --]
[-- Type: text/x-patch; name="decouple-spinlock-dbg-from-stats.patch", Size: 5093 bytes --]
---
include/asm-generic/system.h | 35 ++++++++++++++++-------------------
ksrc/nucleus/module.c | 22 +++++++++++-----------
2 files changed, 27 insertions(+), 30 deletions(-)
Index: xenomai/include/asm-generic/system.h
===================================================================
--- xenomai.orig/include/asm-generic/system.h
+++ xenomai/include/asm-generic/system.h
@@ -71,9 +71,7 @@ typedef unsigned long spl_t;
#define spltest() rthal_local_irq_test()
#define splget(x) rthal_local_irq_flags(x)
-#if defined(CONFIG_SMP) && \
- (defined(CONFIG_XENO_OPT_STATS) || defined(CONFIG_XENO_OPT_DEBUG))
-
+#if defined(CONFIG_SMP) && defined(CONFIG_XENO_OPT_DEBUG)
typedef struct {
unsigned long long spin_time;
@@ -96,25 +94,24 @@ typedef struct {
} xnlock_t;
-#define XNARCH_LOCK_UNLOCKED (xnlock_t) { \
- { ~0 }, \
- NULL, \
- NULL, \
- 0, \
- -1, \
- 0LL, \
- 0LL, \
+#define XNARCH_LOCK_UNLOCKED (xnlock_t) { \
+ { ~0 }, \
+ NULL, \
+ NULL, \
+ 0, \
+ -1, \
+ 0LL, \
+ 0LL, \
}
#define CONFIG_XENO_SPINLOCK_DEBUG 1
-#else /* !(CONFIG_XENO_OPT_STATS && CONFIG_SMP) */
+#else /* !(CONFIG_SMP && CONFIG_XENO_OPT_DEBUG) */
typedef struct { atomic_t owner; } xnlock_t;
#define XNARCH_LOCK_UNLOCKED (xnlock_t) { { ~0 } }
-
-#endif /* CONFIG_XENO_OPT_STATS && CONFIG_SMP */
+#endif /* CONFIG_SMP && CONFIG_XENO_OPT_DEBUG */
#define XNARCH_NR_CPUS RTHAL_NR_CPUS
@@ -253,7 +250,7 @@ static inline int __xnlock_get (xnlock_t
#else /* !CONFIG_XENO_SPINLOCK_DEBUG */
static inline int __xnlock_get (xnlock_t *lock)
{
-#endif /* !CONFIG_XENO_SPINLOCK_DEBUG */
+#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
rthal_declare_cpuid;
int recursing;
@@ -304,7 +301,7 @@ static inline void xnlock_put (xnlock_t
rthal_load_cpuid();
if (likely(atomic_read(&lock->owner) == cpuid)) {
-#ifdef CONFIG_XENO_OPT_STATS
+#ifdef CONFIG_XENO_SPINLOCK_DEBUG
extern xnlockinfo_t xnlock_stats[];
unsigned long long lock_time = rthal_rdtsc() - lock->lock_date;
@@ -316,7 +313,7 @@ static inline void xnlock_put (xnlock_t
xnlock_stats[cpuid].function = lock->function;
xnlock_stats[cpuid].line = lock->line;
}
-#endif /* CONFIG_XENO_OPT_STATS */
+#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
atomic_set(&lock->owner, ~0);
}
#ifdef CONFIG_XENO_SPINLOCK_DEBUG
@@ -343,7 +340,7 @@ static inline spl_t __xnlock_get_irqsave
#else /* !CONFIG_XENO_SPINLOCK_DEBUG */
static inline spl_t __xnlock_get_irqsave (xnlock_t *lock)
{
-#endif /* !CONFIG_XENO_SPINLOCK_DEBUG */
+#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
unsigned long flags;
rthal_local_irq_save(flags);
@@ -354,7 +351,7 @@ static inline spl_t __xnlock_get_irqsave
#else /* !CONFIG_XENO_SPINLOCK_DEBUG */
if (__xnlock_get(lock))
flags |= 2;
-#endif /* !CONFIG_XENO_SPINLOCK_DEBUG */
+#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
return flags;
}
Index: xenomai/ksrc/nucleus/module.c
===================================================================
--- xenomai.orig/ksrc/nucleus/module.c
+++ xenomai/ksrc/nucleus/module.c
@@ -472,7 +472,9 @@ static struct file_operations stat_seq_o
.release = seq_release_private,
};
-#ifdef CONFIG_SMP
+#endif /* CONFIG_XENO_OPT_STATS */
+
+#ifdef CONFIG_XENO_SPINLOCK_DEBUG
xnlockinfo_t xnlock_stats[RTHAL_NR_CPUS];
@@ -520,9 +522,7 @@ static int lock_read_proc(char *page,
EXPORT_SYMBOL(xnlock_stats);
-#endif /* CONFIG_SMP */
-
-#endif /* CONFIG_XENO_OPT_STATS */
+#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
static int latency_read_proc(char *page,
char **start,
@@ -741,12 +741,12 @@ void xnpod_init_proc(void)
#ifdef CONFIG_XENO_OPT_STATS
add_proc_fops("stat", &stat_seq_operations, 0, rthal_proc_root);
-#ifdef CONFIG_SMP
- add_proc_leaf("lock", &lock_read_proc, NULL, NULL, rthal_proc_root);
-#endif /* CONFIG_SMP */
-
#endif /* CONFIG_XENO_OPT_STATS */
+#ifdef CONFIG_XENO_SPINLOCK_DEBUG
+ add_proc_leaf("lock", &lock_read_proc, NULL, NULL, rthal_proc_root);
+#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
+
add_proc_leaf("latency",
&latency_read_proc,
&latency_write_proc, NULL, rthal_proc_root);
@@ -786,10 +786,10 @@ void xnpod_delete_proc(void)
remove_proc_entry("sched", rthal_proc_root);
#ifdef CONFIG_XENO_OPT_STATS
remove_proc_entry("stat", rthal_proc_root);
-#ifdef CONFIG_SMP
- remove_proc_entry("lock", rthal_proc_root);
-#endif /* CONFIG_SMP */
#endif /* CONFIG_XENO_OPT_STATS */
+#ifdef CONFIG_XENO_SPINLOCK_DEBUG
+ remove_proc_entry("lock", rthal_proc_root);
+#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
}
#ifdef CONFIG_XENO_OPT_PERVASIVE
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Xenomai-core] [PATCH 1/3] decouple spinlock stats from XENO_OPT_STATS
2006-11-20 23:14 [Xenomai-core] [PATCH 1/3] decouple spinlock stats from XENO_OPT_STATS Jan Kiszka
@ 2006-11-21 16:40 ` Gilles Chanteperdrix
2006-11-21 17:09 ` Jan Kiszka
2006-11-22 7:20 ` Jan Kiszka
1 sibling, 1 reply; 7+ messages in thread
From: Gilles Chanteperdrix @ 2006-11-21 16:40 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai-core
Jan Kiszka wrote:
> #ifdef CONFIG_XENO_SPINLOCK_DEBUG
> @@ -343,7 +340,7 @@ static inline spl_t __xnlock_get_irqsave
> #else /* !CONFIG_XENO_SPINLOCK_DEBUG */
> static inline spl_t __xnlock_get_irqsave (xnlock_t *lock)
> {
> -#endif /* !CONFIG_XENO_SPINLOCK_DEBUG */
> +#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
I prefer the notation:
#if cond
#endif /* cond */
and
#if cond
#else /* !cond */
#endif /* !cond */
to
#if cond
#endif /* cond */
and
#if cond
#else /* !cond */
#endif /* cond */
Because in the first case, when I see a #endif in the middle of a lot of
code, I immediately know what predicate is true above the #endif.
--
Gilles Chanteperdrix
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Xenomai-core] [PATCH 1/3] decouple spinlock stats from XENO_OPT_STATS
2006-11-21 16:40 ` Gilles Chanteperdrix
@ 2006-11-21 17:09 ` Jan Kiszka
2006-11-21 17:28 ` Philippe Gerum
0 siblings, 1 reply; 7+ messages in thread
From: Jan Kiszka @ 2006-11-21 17:09 UTC (permalink / raw)
To: Gilles Chanteperdrix; +Cc: xenomai-core
[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]
Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> #ifdef CONFIG_XENO_SPINLOCK_DEBUG
>> @@ -343,7 +340,7 @@ static inline spl_t __xnlock_get_irqsave
>> #else /* !CONFIG_XENO_SPINLOCK_DEBUG */
>> static inline spl_t __xnlock_get_irqsave (xnlock_t *lock)
>> {
>> -#endif /* !CONFIG_XENO_SPINLOCK_DEBUG */
>> +#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
>
> I prefer the notation:
>
> #if cond
> #endif /* cond */
>
> and
>
> #if cond
> #else /* !cond */
> #endif /* !cond */
>
> to
>
> #if cond
> #endif /* cond */
>
> and
>
> #if cond
> #else /* !cond */
> #endif /* cond */
>
> Because in the first case, when I see a #endif in the middle of a lot of
> code, I immediately know what predicate is true above the #endif.
>
Ok, I understand the logic. Nevertheless, I had the impression the first
scheme is used far more often than the second one in Xenomai and I-pipe,
so I took the chance to consolidate the code.
So, what should be the common rule from now on? Is there a kernel-style
rule for this issue we should follow, or are we free and should convert
to your preferred scheme?
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Xenomai-core] [PATCH 1/3] decouple spinlock stats from XENO_OPT_STATS
2006-11-21 17:09 ` Jan Kiszka
@ 2006-11-21 17:28 ` Philippe Gerum
0 siblings, 0 replies; 7+ messages in thread
From: Philippe Gerum @ 2006-11-21 17:28 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai-core
On Tue, 2006-11-21 at 18:09 +0100, Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
> > Jan Kiszka wrote:
> >> #ifdef CONFIG_XENO_SPINLOCK_DEBUG
> >> @@ -343,7 +340,7 @@ static inline spl_t __xnlock_get_irqsave
> >> #else /* !CONFIG_XENO_SPINLOCK_DEBUG */
> >> static inline spl_t __xnlock_get_irqsave (xnlock_t *lock)
> >> {
> >> -#endif /* !CONFIG_XENO_SPINLOCK_DEBUG */
> >> +#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
> >
> > I prefer the notation:
> >
> > #if cond
> > #endif /* cond */
> >
> > and
> >
> > #if cond
> > #else /* !cond */
> > #endif /* !cond */
> >
> > to
> >
> > #if cond
> > #endif /* cond */
> >
> > and
> >
> > #if cond
> > #else /* !cond */
> > #endif /* cond */
> >
> > Because in the first case, when I see a #endif in the middle of a lot of
> > code, I immediately know what predicate is true above the #endif.
> >
>
> Ok, I understand the logic. Nevertheless, I had the impression the first
> scheme is used far more often than the second one in Xenomai and I-pipe,
> so I took the chance to consolidate the code.
Basically because I wrote those using the first convention.
>
> So, what should be the common rule from now on? Is there a kernel-style
> rule for this issue we should follow, or are we free and should convert
> to your preferred scheme?
>
Let's move to Gilles's convention; it's more readable.
> Jan
>
> _______________________________________________
> Xenomai-core mailing list
> Xenomai-core@domain.hid
> https://mail.gna.org/listinfo/xenomai-core
--
Philippe.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Xenomai-core] [PATCH 1/3] decouple spinlock stats from XENO_OPT_STATS
2006-11-20 23:14 [Xenomai-core] [PATCH 1/3] decouple spinlock stats from XENO_OPT_STATS Jan Kiszka
2006-11-21 16:40 ` Gilles Chanteperdrix
@ 2006-11-22 7:20 ` Jan Kiszka
2006-12-02 18:29 ` Philippe Gerum
1 sibling, 1 reply; 7+ messages in thread
From: Jan Kiszka @ 2006-11-22 7:20 UTC (permalink / raw)
To: xenomai-core
[-- Attachment #1: Type: text/plain, Size: 95 bytes --]
Here comes version 2 of this patch, now following the new
#if-#else-#endif comment style.
Jan
[-- Attachment #2: decouple-spinlock-dbg-from-stats-v2.patch --]
[-- Type: text/x-patch, Size: 5107 bytes --]
---
include/asm-generic/system.h | 35 ++++++++++++++++-------------------
ksrc/nucleus/module.c | 22 +++++++++++-----------
2 files changed, 27 insertions(+), 30 deletions(-)
Index: xenomai/include/asm-generic/system.h
===================================================================
--- xenomai.orig/include/asm-generic/system.h
+++ xenomai/include/asm-generic/system.h
@@ -66,14 +66,12 @@ typedef unsigned long spl_t;
#define splexit(x) rthal_local_irq_restore((x) & 1)
#else /* !CONFIG_SMP */
#define splexit(x) rthal_local_irq_restore(x)
-#endif /* CONFIG_SMP */
+#endif /* !CONFIG_SMP */
#define splnone() rthal_local_irq_enable()
#define spltest() rthal_local_irq_test()
#define splget(x) rthal_local_irq_flags(x)
-#if defined(CONFIG_SMP) && \
- (defined(CONFIG_XENO_OPT_STATS) || defined(CONFIG_XENO_OPT_DEBUG))
-
+#if defined(CONFIG_SMP) && defined(CONFIG_XENO_OPT_DEBUG)
typedef struct {
unsigned long long spin_time;
@@ -96,25 +94,24 @@ typedef struct {
} xnlock_t;
-#define XNARCH_LOCK_UNLOCKED (xnlock_t) { \
- { ~0 }, \
- NULL, \
- NULL, \
- 0, \
- -1, \
- 0LL, \
- 0LL, \
+#define XNARCH_LOCK_UNLOCKED (xnlock_t) { \
+ { ~0 }, \
+ NULL, \
+ NULL, \
+ 0, \
+ -1, \
+ 0LL, \
+ 0LL, \
}
#define CONFIG_XENO_SPINLOCK_DEBUG 1
-#else /* !(CONFIG_XENO_OPT_STATS && CONFIG_SMP) */
+#else /* !(CONFIG_SMP && CONFIG_XENO_OPT_DEBUG) */
typedef struct { atomic_t owner; } xnlock_t;
#define XNARCH_LOCK_UNLOCKED (xnlock_t) { { ~0 } }
-
-#endif /* CONFIG_XENO_OPT_STATS && CONFIG_SMP */
+#endif /* !(CONFIG_SMP && CONFIG_XENO_OPT_DEBUG) */
#define XNARCH_NR_CPUS RTHAL_NR_CPUS
@@ -231,7 +228,7 @@ static inline int xnarch_setimask (int i
#else /* !CONFIG_XENO_SPINLOCK_DEBUG */
#define xnlock_get(lock) __xnlock_get(lock)
#define xnlock_get_irqsave(lock,x) ((x) = __xnlock_get_irqsave(lock))
-#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
+#endif /* !CONFIG_XENO_SPINLOCK_DEBUG */
#define xnlock_clear_irqoff(lock) xnlock_put_irqrestore(lock,1)
#define xnlock_clear_irqon(lock) xnlock_put_irqrestore(lock,0)
@@ -304,7 +301,7 @@ static inline void xnlock_put (xnlock_t
rthal_load_cpuid();
if (likely(atomic_read(&lock->owner) == cpuid)) {
-#ifdef CONFIG_XENO_OPT_STATS
+#ifdef CONFIG_XENO_SPINLOCK_DEBUG
extern xnlockinfo_t xnlock_stats[];
unsigned long long lock_time = rthal_rdtsc() - lock->lock_date;
@@ -316,7 +313,7 @@ static inline void xnlock_put (xnlock_t
xnlock_stats[cpuid].function = lock->function;
xnlock_stats[cpuid].line = lock->line;
}
-#endif /* CONFIG_XENO_OPT_STATS */
+#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
atomic_set(&lock->owner, ~0);
}
#ifdef CONFIG_XENO_SPINLOCK_DEBUG
@@ -377,7 +374,7 @@ static inline void xnlock_put_irqrestore
#define xnlock_clear_irqoff(lock) rthal_local_irq_disable()
#define xnlock_clear_irqon(lock) rthal_local_irq_enable()
-#endif /* CONFIG_SMP */
+#endif /* !CONFIG_SMP */
static inline int xnarch_remap_vm_page(struct vm_area_struct *vma,
unsigned long from,
Index: xenomai/ksrc/nucleus/module.c
===================================================================
--- xenomai.orig/ksrc/nucleus/module.c
+++ xenomai/ksrc/nucleus/module.c
@@ -472,7 +472,9 @@ static struct file_operations stat_seq_o
.release = seq_release_private,
};
-#ifdef CONFIG_SMP
+#endif /* CONFIG_XENO_OPT_STATS */
+
+#ifdef CONFIG_XENO_SPINLOCK_DEBUG
xnlockinfo_t xnlock_stats[RTHAL_NR_CPUS];
@@ -520,9 +522,7 @@ static int lock_read_proc(char *page,
EXPORT_SYMBOL(xnlock_stats);
-#endif /* CONFIG_SMP */
-
-#endif /* CONFIG_XENO_OPT_STATS */
+#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
static int latency_read_proc(char *page,
char **start,
@@ -741,12 +741,12 @@ void xnpod_init_proc(void)
#ifdef CONFIG_XENO_OPT_STATS
add_proc_fops("stat", &stat_seq_operations, 0, rthal_proc_root);
-#ifdef CONFIG_SMP
- add_proc_leaf("lock", &lock_read_proc, NULL, NULL, rthal_proc_root);
-#endif /* CONFIG_SMP */
-
#endif /* CONFIG_XENO_OPT_STATS */
+#ifdef CONFIG_XENO_SPINLOCK_DEBUG
+ add_proc_leaf("lock", &lock_read_proc, NULL, NULL, rthal_proc_root);
+#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
+
add_proc_leaf("latency",
&latency_read_proc,
&latency_write_proc, NULL, rthal_proc_root);
@@ -786,10 +786,10 @@ void xnpod_delete_proc(void)
remove_proc_entry("sched", rthal_proc_root);
#ifdef CONFIG_XENO_OPT_STATS
remove_proc_entry("stat", rthal_proc_root);
-#ifdef CONFIG_SMP
- remove_proc_entry("lock", rthal_proc_root);
-#endif /* CONFIG_SMP */
#endif /* CONFIG_XENO_OPT_STATS */
+#ifdef CONFIG_XENO_SPINLOCK_DEBUG
+ remove_proc_entry("lock", rthal_proc_root);
+#endif /* CONFIG_XENO_SPINLOCK_DEBUG */
}
#ifdef CONFIG_XENO_OPT_PERVASIVE
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Xenomai-core] [PATCH 1/3] decouple spinlock stats from XENO_OPT_STATS
2006-11-22 7:20 ` Jan Kiszka
@ 2006-12-02 18:29 ` Philippe Gerum
2006-12-04 9:20 ` Gilles Chanteperdrix
0 siblings, 1 reply; 7+ messages in thread
From: Philippe Gerum @ 2006-12-02 18:29 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai-core
Ok, I ended up finding such decoupling cleaner, given that we don't drag
the full debug overhead when activating /proc/xenomai/locks in SMP mode,
thanks to the reorganized debug options. Gilles, is this patch series ok
for you too, and particularly the POSIX changes?
--
Philippe.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Xenomai-core] [PATCH 1/3] decouple spinlock stats from XENO_OPT_STATS
2006-12-02 18:29 ` Philippe Gerum
@ 2006-12-04 9:20 ` Gilles Chanteperdrix
0 siblings, 0 replies; 7+ messages in thread
From: Gilles Chanteperdrix @ 2006-12-04 9:20 UTC (permalink / raw)
To: rpm; +Cc: Jan Kiszka, xenomai-core
Philippe Gerum wrote:
> Ok, I ended up finding such decoupling cleaner, given that we don't drag
> the full debug overhead when activating /proc/xenomai/locks in SMP mode,
> thanks to the reorganized debug options. Gilles, is this patch series ok
> for you too, and particularly the POSIX changes?
>
Yes, it is Ok.
--
Gilles Chanteperdrix
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2006-12-04 9:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-11-20 23:14 [Xenomai-core] [PATCH 1/3] decouple spinlock stats from XENO_OPT_STATS Jan Kiszka
2006-11-21 16:40 ` Gilles Chanteperdrix
2006-11-21 17:09 ` Jan Kiszka
2006-11-21 17:28 ` Philippe Gerum
2006-11-22 7:20 ` Jan Kiszka
2006-12-02 18:29 ` Philippe Gerum
2006-12-04 9:20 ` Gilles Chanteperdrix
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.