All of lore.kernel.org
 help / color / mirror / Atom feed
* [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
@ 2018-09-04 13:31 Philippe Gerum
  2018-09-04 17:48 ` Jan Kiszka
  2018-09-24 13:52 ` Jan Kiszka
  0 siblings, 2 replies; 14+ messages in thread
From: Philippe Gerum @ 2018-09-04 13:31 UTC (permalink / raw)
  To: xenomai

Testing debug switches using '#if XENO_DEBUG(<feat>)' may confuse the
build system's dependency tracker, breaking the build, or even
producing a non-bootable kernel in extreme cases. Typically, turning
on/off the lock debugging code - usually in absence of any other
change - may cause the following error to pop up at the link stage:

---
MODPOST vmlinux.o
kernel/xenomai/lock.o: In function `____xnlock_get':
linux/include/xenomai/cobalt/kernel/lock.h:184: undefined reference to `xnlock_dbg_prepare_acquire'
linux/include/xenomai/cobalt/kernel/lock.h:189: undefined reference to `xnlock_dbg_acquired'
kernel/xenomai/lock.o: In function `____xnlock_put':
linux/include/xenomai/cobalt/kernel/lock.h:196: undefined reference to `xnlock_dbg_release'
---

Replace all occurrences of the confusing conditionals with the
equivalent expression explicitly testing the debug symbol.

Signed-off-by: Philippe Gerum <rpm@xenomai.org>
---
 include/cobalt/kernel/assert.h         |  2 +-
 include/cobalt/kernel/lock.h           |  6 +++---
 include/cobalt/kernel/sched-sporadic.h |  2 +-
 include/cobalt/kernel/synch.h          |  2 +-
 include/cobalt/kernel/tree.h           |  2 +-
 kernel/cobalt/bufd.c                   | 10 +++++-----
 kernel/cobalt/debug.c                  |  2 +-
 kernel/cobalt/intr.c                   |  2 +-
 kernel/cobalt/posix/process.c          |  2 +-
 kernel/cobalt/procfs.c                 |  2 +-
 kernel/cobalt/sched-sporadic.c         |  4 ++--
 kernel/cobalt/synch.c                  |  2 +-
 12 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/include/cobalt/kernel/assert.h b/include/cobalt/kernel/assert.h
index 2d2d65396..86d0a480f 100644
--- a/include/cobalt/kernel/assert.h
+++ b/include/cobalt/kernel/assert.h
@@ -63,7 +63,7 @@
 #define realtime_cpu_only()	XENO_BUG_ON(CONTEXT, !xnsched_supported_cpu(ipipe_processor_id()))
 #define thread_only()		XENO_BUG_ON(CONTEXT, xnsched_interrupt_p())
 #define irqoff_only()		XENO_BUG_ON(CONTEXT, hard_irqs_disabled() == 0)
-#if XENO_DEBUG(LOCKING)
+#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
 #define atomic_only()		XENO_BUG_ON(CONTEXT, (xnlock_is_owner(&nklock) && hard_irqs_disabled()) == 0)
 #define preemptible_only()	XENO_BUG_ON(CONTEXT, xnlock_is_owner(&nklock) || hard_irqs_disabled())
 #else
diff --git a/include/cobalt/kernel/lock.h b/include/cobalt/kernel/lock.h
index 36f81688a..99c7848e8 100644
--- a/include/cobalt/kernel/lock.h
+++ b/include/cobalt/kernel/lock.h
@@ -63,7 +63,7 @@ typedef unsigned long spl_t;
  */
 #define spltest()   ipipe_test_head()
 
-#if XENO_DEBUG(LOCKING)
+#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
 
 struct xnlock {
 	unsigned owner;
@@ -152,7 +152,7 @@ static inline int xnlock_dbg_release(struct xnlock *lock)
 
 #endif /* !XENO_DEBUG(LOCKING) */
 
-#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)
+#if defined(CONFIG_SMP) || defined(CONFIG_XENO_OPT_DEBUG_LOCKING)
 
 #define xnlock_get(lock)		__xnlock_get(lock  XNLOCK_DBG_CONTEXT)
 #define xnlock_put(lock)		__xnlock_put(lock  XNLOCK_DBG_CONTEXT)
@@ -209,7 +209,7 @@ int ___xnlock_get(struct xnlock *lock /*, */ XNLOCK_DBG_CONTEXT_ARGS);
 void ___xnlock_put(struct xnlock *lock /*, */ XNLOCK_DBG_CONTEXT_ARGS);
 #endif /* out of line xnlock */
 
-#if XENO_DEBUG(LOCKING)
+#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
 /* Disable UP-over-SMP kernel optimization in debug mode. */
 #define __locking_active__  1
 #else
diff --git a/include/cobalt/kernel/sched-sporadic.h b/include/cobalt/kernel/sched-sporadic.h
index 0eade47b0..50ca406c4 100644
--- a/include/cobalt/kernel/sched-sporadic.h
+++ b/include/cobalt/kernel/sched-sporadic.h
@@ -56,7 +56,7 @@ struct xnsched_sporadic_data {
 };
 
 struct xnsched_sporadic {
-#if XENO_DEBUG(COBALT)
+#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
 	unsigned long drop_retries;
 #endif
 };
diff --git a/include/cobalt/kernel/synch.h b/include/cobalt/kernel/synch.h
index f5ac217ed..08056055a 100644
--- a/include/cobalt/kernel/synch.h
+++ b/include/cobalt/kernel/synch.h
@@ -101,7 +101,7 @@ static inline struct xnthread *xnsynch_owner(struct xnsynch *synch)
 #define xnsynch_owner_check(synch, thread) \
 	xnsynch_fast_owner_check((synch)->fastlock, thread->handle)
 
-#if XENO_DEBUG(MUTEX_RELAXED)
+#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
 
 void xnsynch_detect_relaxed_owner(struct xnsynch *synch,
 				  struct xnthread *sleeper);
diff --git a/include/cobalt/kernel/tree.h b/include/cobalt/kernel/tree.h
index 9c751cc02..c52ee3220 100644
--- a/include/cobalt/kernel/tree.h
+++ b/include/cobalt/kernel/tree.h
@@ -83,7 +83,7 @@ struct xnid *xnid_fetch(struct rb_root *t, xnkey_t key)
 
 static inline int xnid_remove(struct rb_root *t, struct xnid *xnid)
 {
-#if XENO_DEBUG(COBALT)
+#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
 	if (xnid_fetch(t, xnid->key) != xnid)
 		return -ENOENT;
 #endif
diff --git a/kernel/cobalt/bufd.c b/kernel/cobalt/bufd.c
index 9b41b6ad6..3b79505d5 100644
--- a/kernel/cobalt/bufd.c
+++ b/kernel/cobalt/bufd.c
@@ -493,7 +493,7 @@ ssize_t xnbufd_unmap_uread(struct xnbufd *bufd)
 {
 	preemptible_only();
 
-#if XENO_DEBUG(COBALT)
+#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
 	bufd->b_ptr = (caddr_t)-1;
 #endif
 	return bufd->b_off;
@@ -551,7 +551,7 @@ ssize_t xnbufd_unmap_uwrite(struct xnbufd *bufd)
 	if (bufd->b_len > sizeof(bufd->b_buf))
 		xnfree(bufd->b_carry);
 done:
-#if XENO_DEBUG(COBALT)
+#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
 	bufd->b_ptr = (caddr_t)-1;
 #endif
 	return ret ?: (ssize_t)len;
@@ -592,7 +592,7 @@ EXPORT_SYMBOL_GPL(xnbufd_unmap_uwrite);
  */
 void xnbufd_invalidate(struct xnbufd *bufd)
 {
-#if XENO_DEBUG(COBALT)
+#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
 	bufd->b_ptr = (caddr_t)-1;
 #endif
 	if (bufd->b_carry) {
@@ -620,7 +620,7 @@ EXPORT_SYMBOL_GPL(xnbufd_invalidate);
  */
 ssize_t xnbufd_unmap_kread(struct xnbufd *bufd)
 {
-#if XENO_DEBUG(COBALT)
+#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
 	bufd->b_ptr = (caddr_t)-1;
 #endif
 	return bufd->b_off;
@@ -643,7 +643,7 @@ EXPORT_SYMBOL_GPL(xnbufd_unmap_kread);
  */
 ssize_t xnbufd_unmap_kwrite(struct xnbufd *bufd)
 {
-#if XENO_DEBUG(COBALT)
+#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
 	bufd->b_ptr = (caddr_t)-1;
 #endif
 	return bufd->b_off;
diff --git a/kernel/cobalt/debug.c b/kernel/cobalt/debug.c
index 74022c516..af9a95ab6 100644
--- a/kernel/cobalt/debug.c
+++ b/kernel/cobalt/debug.c
@@ -549,7 +549,7 @@ static inline void init_thread_relax_trace(struct xnthread *thread)
 
 #endif /* !XENO_OPT_DEBUG_TRACE_RELAX */
 
-#if XENO_DEBUG(LOCKING)
+#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
 
 void xnlock_dbg_prepare_acquire(unsigned long long *start)
 {
diff --git a/kernel/cobalt/intr.c b/kernel/cobalt/intr.c
index fb27a3295..69faa23ba 100644
--- a/kernel/cobalt/intr.c
+++ b/kernel/cobalt/intr.c
@@ -531,7 +531,7 @@ static inline void xnintr_irq_detach(struct xnintr *intr)
 #else /* !CONFIG_XENO_OPT_SHIRQ */
 
 struct xnintr_vector {
-#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)
+#if defined(CONFIG_SMP) || defined(CONFIG_XENO_OPT_DEBUG_LOCKING)
 	DECLARE_XNLOCK(lock);
 #endif /* CONFIG_SMP || XENO_DEBUG(LOCKING) */
 } ____cacheline_aligned_in_smp;
diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
index dbcb81ef8..50f752f13 100644
--- a/kernel/cobalt/posix/process.c
+++ b/kernel/cobalt/posix/process.c
@@ -751,7 +751,7 @@ static inline int handle_exception(struct ipipe_trap_data *d)
 	 * running in primary mode, move it to the Linux domain,
 	 * leaving the kernel process the exception.
 	 */
-#if XENO_DEBUG(COBALT) || XENO_DEBUG(USER)
+#if defined(CONFIG_XENO_OPT_DEBUG_COBALT) || defined(CONFIG_XENO_OPT_DEBUG_USER)
 	if (!user_mode(d->regs)) {
 		xntrace_panic_freeze();
 		printk(XENO_WARNING
diff --git a/kernel/cobalt/procfs.c b/kernel/cobalt/procfs.c
index ade13cf1c..d1f01f038 100644
--- a/kernel/cobalt/procfs.c
+++ b/kernel/cobalt/procfs.c
@@ -27,7 +27,7 @@
 #include <xenomai/version.h>
 #include "debug.h"
 
-#if XENO_DEBUG(LOCKING)
+#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
 
 static int lock_vfile_show(struct xnvfile_regular_iterator *it, void *data)
 {
diff --git a/kernel/cobalt/sched-sporadic.c b/kernel/cobalt/sched-sporadic.c
index d2a1f76d0..9cbfa5be3 100644
--- a/kernel/cobalt/sched-sporadic.c
+++ b/kernel/cobalt/sched-sporadic.c
@@ -24,7 +24,7 @@
 
 static void sporadic_post_recharge(struct xnthread *thread, xnticks_t budget);
 
-#if XENO_DEBUG(COBALT)
+#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
 
 static inline void sporadic_note_late_drop(struct xnsched *sched)
 {
@@ -236,7 +236,7 @@ static void xnsched_sporadic_init(struct xnsched *sched)
 	 * share the same priority scale, with the addition of budget
 	 * management for the sporadic ones.
 	 */
-#if XENO_DEBUG(COBALT)
+#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
 	sched->pss.drop_retries = 0;
 #endif
 }
diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
index 7773a08e5..32df635a8 100644
--- a/kernel/cobalt/synch.c
+++ b/kernel/cobalt/synch.c
@@ -902,7 +902,7 @@ void xnsynch_release_all_ownerships(struct xnthread *thread)
 }
 EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships);
 
-#if XENO_DEBUG(MUTEX_RELAXED)
+#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
 
 /*
  * Detect when a thread is about to sleep on a synchronization
-- 
2.14.4



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
  2018-09-04 13:31 [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals Philippe Gerum
@ 2018-09-04 17:48 ` Jan Kiszka
  2018-09-05  7:44   ` Philippe Gerum
  2018-09-05  8:16   ` Philippe Gerum
  2018-09-24 13:52 ` Jan Kiszka
  1 sibling, 2 replies; 14+ messages in thread
From: Jan Kiszka @ 2018-09-04 17:48 UTC (permalink / raw)
  To: Philippe Gerum, xenomai

On 2018-09-04 15:31, Philippe Gerum wrote:
> Testing debug switches using '#if XENO_DEBUG(<feat>)' may confuse the
> build system's dependency tracker, breaking the build, or even
> producing a non-bootable kernel in extreme cases. Typically, turning
> on/off the lock debugging code - usually in absence of any other
> change - may cause the following error to pop up at the link stage:
> 
> ---
> MODPOST vmlinux.o
> kernel/xenomai/lock.o: In function `____xnlock_get':
> linux/include/xenomai/cobalt/kernel/lock.h:184: undefined reference to `xnlock_dbg_prepare_acquire'
> linux/include/xenomai/cobalt/kernel/lock.h:189: undefined reference to `xnlock_dbg_acquired'
> kernel/xenomai/lock.o: In function `____xnlock_put':
> linux/include/xenomai/cobalt/kernel/lock.h:196: undefined reference to `xnlock_dbg_release'
> ---
> 
> Replace all occurrences of the confusing conditionals with the
> equivalent expression explicitly testing the debug symbol.
> 
> Signed-off-by: Philippe Gerum <rpm@xenomai.org>
> ---
>   include/cobalt/kernel/assert.h         |  2 +-
>   include/cobalt/kernel/lock.h           |  6 +++---
>   include/cobalt/kernel/sched-sporadic.h |  2 +-
>   include/cobalt/kernel/synch.h          |  2 +-
>   include/cobalt/kernel/tree.h           |  2 +-
>   kernel/cobalt/bufd.c                   | 10 +++++-----
>   kernel/cobalt/debug.c                  |  2 +-
>   kernel/cobalt/intr.c                   |  2 +-
>   kernel/cobalt/posix/process.c          |  2 +-
>   kernel/cobalt/procfs.c                 |  2 +-
>   kernel/cobalt/sched-sporadic.c         |  4 ++--
>   kernel/cobalt/synch.c                  |  2 +-
>   12 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/include/cobalt/kernel/assert.h b/include/cobalt/kernel/assert.h
> index 2d2d65396..86d0a480f 100644
> --- a/include/cobalt/kernel/assert.h
> +++ b/include/cobalt/kernel/assert.h
> @@ -63,7 +63,7 @@
>   #define realtime_cpu_only()	XENO_BUG_ON(CONTEXT, !xnsched_supported_cpu(ipipe_processor_id()))
>   #define thread_only()		XENO_BUG_ON(CONTEXT, xnsched_interrupt_p())
>   #define irqoff_only()		XENO_BUG_ON(CONTEXT, hard_irqs_disabled() == 0)
> -#if XENO_DEBUG(LOCKING)
> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>   #define atomic_only()		XENO_BUG_ON(CONTEXT, (xnlock_is_owner(&nklock) && hard_irqs_disabled()) == 0)
>   #define preemptible_only()	XENO_BUG_ON(CONTEXT, xnlock_is_owner(&nklock) || hard_irqs_disabled())
>   #else
> diff --git a/include/cobalt/kernel/lock.h b/include/cobalt/kernel/lock.h
> index 36f81688a..99c7848e8 100644
> --- a/include/cobalt/kernel/lock.h
> +++ b/include/cobalt/kernel/lock.h
> @@ -63,7 +63,7 @@ typedef unsigned long spl_t;
>    */
>   #define spltest()   ipipe_test_head()
>   
> -#if XENO_DEBUG(LOCKING)
> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>   
>   struct xnlock {
>   	unsigned owner;
> @@ -152,7 +152,7 @@ static inline int xnlock_dbg_release(struct xnlock *lock)
>   
>   #endif /* !XENO_DEBUG(LOCKING) */
>   
> -#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)
> +#if defined(CONFIG_SMP) || defined(CONFIG_XENO_OPT_DEBUG_LOCKING)
>   
>   #define xnlock_get(lock)		__xnlock_get(lock  XNLOCK_DBG_CONTEXT)
>   #define xnlock_put(lock)		__xnlock_put(lock  XNLOCK_DBG_CONTEXT)
> @@ -209,7 +209,7 @@ int ___xnlock_get(struct xnlock *lock /*, */ XNLOCK_DBG_CONTEXT_ARGS);
>   void ___xnlock_put(struct xnlock *lock /*, */ XNLOCK_DBG_CONTEXT_ARGS);
>   #endif /* out of line xnlock */
>   
> -#if XENO_DEBUG(LOCKING)
> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>   /* Disable UP-over-SMP kernel optimization in debug mode. */
>   #define __locking_active__  1
>   #else
> diff --git a/include/cobalt/kernel/sched-sporadic.h b/include/cobalt/kernel/sched-sporadic.h
> index 0eade47b0..50ca406c4 100644
> --- a/include/cobalt/kernel/sched-sporadic.h
> +++ b/include/cobalt/kernel/sched-sporadic.h
> @@ -56,7 +56,7 @@ struct xnsched_sporadic_data {
>   };
>   
>   struct xnsched_sporadic {
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	unsigned long drop_retries;
>   #endif
>   };
> diff --git a/include/cobalt/kernel/synch.h b/include/cobalt/kernel/synch.h
> index f5ac217ed..08056055a 100644
> --- a/include/cobalt/kernel/synch.h
> +++ b/include/cobalt/kernel/synch.h
> @@ -101,7 +101,7 @@ static inline struct xnthread *xnsynch_owner(struct xnsynch *synch)
>   #define xnsynch_owner_check(synch, thread) \
>   	xnsynch_fast_owner_check((synch)->fastlock, thread->handle)
>   
> -#if XENO_DEBUG(MUTEX_RELAXED)
> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>   
>   void xnsynch_detect_relaxed_owner(struct xnsynch *synch,
>   				  struct xnthread *sleeper);
> diff --git a/include/cobalt/kernel/tree.h b/include/cobalt/kernel/tree.h
> index 9c751cc02..c52ee3220 100644
> --- a/include/cobalt/kernel/tree.h
> +++ b/include/cobalt/kernel/tree.h
> @@ -83,7 +83,7 @@ struct xnid *xnid_fetch(struct rb_root *t, xnkey_t key)
>   
>   static inline int xnid_remove(struct rb_root *t, struct xnid *xnid)
>   {
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	if (xnid_fetch(t, xnid->key) != xnid)
>   		return -ENOENT;
>   #endif
> diff --git a/kernel/cobalt/bufd.c b/kernel/cobalt/bufd.c
> index 9b41b6ad6..3b79505d5 100644
> --- a/kernel/cobalt/bufd.c
> +++ b/kernel/cobalt/bufd.c
> @@ -493,7 +493,7 @@ ssize_t xnbufd_unmap_uread(struct xnbufd *bufd)
>   {
>   	preemptible_only();
>   
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	bufd->b_ptr = (caddr_t)-1;
>   #endif
>   	return bufd->b_off;
> @@ -551,7 +551,7 @@ ssize_t xnbufd_unmap_uwrite(struct xnbufd *bufd)
>   	if (bufd->b_len > sizeof(bufd->b_buf))
>   		xnfree(bufd->b_carry);
>   done:
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	bufd->b_ptr = (caddr_t)-1;
>   #endif
>   	return ret ?: (ssize_t)len;
> @@ -592,7 +592,7 @@ EXPORT_SYMBOL_GPL(xnbufd_unmap_uwrite);
>    */
>   void xnbufd_invalidate(struct xnbufd *bufd)
>   {
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	bufd->b_ptr = (caddr_t)-1;
>   #endif
>   	if (bufd->b_carry) {
> @@ -620,7 +620,7 @@ EXPORT_SYMBOL_GPL(xnbufd_invalidate);
>    */
>   ssize_t xnbufd_unmap_kread(struct xnbufd *bufd)
>   {
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	bufd->b_ptr = (caddr_t)-1;
>   #endif
>   	return bufd->b_off;
> @@ -643,7 +643,7 @@ EXPORT_SYMBOL_GPL(xnbufd_unmap_kread);
>    */
>   ssize_t xnbufd_unmap_kwrite(struct xnbufd *bufd)
>   {
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	bufd->b_ptr = (caddr_t)-1;
>   #endif
>   	return bufd->b_off;
> diff --git a/kernel/cobalt/debug.c b/kernel/cobalt/debug.c
> index 74022c516..af9a95ab6 100644
> --- a/kernel/cobalt/debug.c
> +++ b/kernel/cobalt/debug.c
> @@ -549,7 +549,7 @@ static inline void init_thread_relax_trace(struct xnthread *thread)
>   
>   #endif /* !XENO_OPT_DEBUG_TRACE_RELAX */
>   
> -#if XENO_DEBUG(LOCKING)
> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>   
>   void xnlock_dbg_prepare_acquire(unsigned long long *start)
>   {
> diff --git a/kernel/cobalt/intr.c b/kernel/cobalt/intr.c
> index fb27a3295..69faa23ba 100644
> --- a/kernel/cobalt/intr.c
> +++ b/kernel/cobalt/intr.c
> @@ -531,7 +531,7 @@ static inline void xnintr_irq_detach(struct xnintr *intr)
>   #else /* !CONFIG_XENO_OPT_SHIRQ */
>   
>   struct xnintr_vector {
> -#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)
> +#if defined(CONFIG_SMP) || defined(CONFIG_XENO_OPT_DEBUG_LOCKING)
>   	DECLARE_XNLOCK(lock);
>   #endif /* CONFIG_SMP || XENO_DEBUG(LOCKING) */
>   } ____cacheline_aligned_in_smp;
> diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
> index dbcb81ef8..50f752f13 100644
> --- a/kernel/cobalt/posix/process.c
> +++ b/kernel/cobalt/posix/process.c
> @@ -751,7 +751,7 @@ static inline int handle_exception(struct ipipe_trap_data *d)
>   	 * running in primary mode, move it to the Linux domain,
>   	 * leaving the kernel process the exception.
>   	 */
> -#if XENO_DEBUG(COBALT) || XENO_DEBUG(USER)
> +#if defined(CONFIG_XENO_OPT_DEBUG_COBALT) || defined(CONFIG_XENO_OPT_DEBUG_USER)
>   	if (!user_mode(d->regs)) {
>   		xntrace_panic_freeze();
>   		printk(XENO_WARNING
> diff --git a/kernel/cobalt/procfs.c b/kernel/cobalt/procfs.c
> index ade13cf1c..d1f01f038 100644
> --- a/kernel/cobalt/procfs.c
> +++ b/kernel/cobalt/procfs.c
> @@ -27,7 +27,7 @@
>   #include <xenomai/version.h>
>   #include "debug.h"
>   
> -#if XENO_DEBUG(LOCKING)
> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>   
>   static int lock_vfile_show(struct xnvfile_regular_iterator *it, void *data)
>   {
> diff --git a/kernel/cobalt/sched-sporadic.c b/kernel/cobalt/sched-sporadic.c
> index d2a1f76d0..9cbfa5be3 100644
> --- a/kernel/cobalt/sched-sporadic.c
> +++ b/kernel/cobalt/sched-sporadic.c
> @@ -24,7 +24,7 @@
>   
>   static void sporadic_post_recharge(struct xnthread *thread, xnticks_t budget);
>   
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   
>   static inline void sporadic_note_late_drop(struct xnsched *sched)
>   {
> @@ -236,7 +236,7 @@ static void xnsched_sporadic_init(struct xnsched *sched)
>   	 * share the same priority scale, with the addition of budget
>   	 * management for the sporadic ones.
>   	 */
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	sched->pss.drop_retries = 0;
>   #endif
>   }
> diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
> index 7773a08e5..32df635a8 100644
> --- a/kernel/cobalt/synch.c
> +++ b/kernel/cobalt/synch.c
> @@ -902,7 +902,7 @@ void xnsynch_release_all_ownerships(struct xnthread *thread)
>   }
>   EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships);
>   
> -#if XENO_DEBUG(MUTEX_RELAXED)
> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>   
>   /*
>    * Detect when a thread is about to sleep on a synchronization
> 

Confused: This is not for next, is it?

E.g. that hunk above does not apply there anymore, for about a year...

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
  2018-09-04 17:48 ` Jan Kiszka
@ 2018-09-05  7:44   ` Philippe Gerum
  2018-09-05  7:48     ` Philippe Gerum
  2018-09-05  8:16   ` Philippe Gerum
  1 sibling, 1 reply; 14+ messages in thread
From: Philippe Gerum @ 2018-09-05  7:44 UTC (permalink / raw)
  To: Jan Kiszka, xenomai

On 09/04/2018 07:48 PM, Jan Kiszka wrote:
> On 2018-09-04 15:31, Philippe Gerum wrote:
>> Testing debug switches using '#if XENO_DEBUG(<feat>)' may confuse the
>> build system's dependency tracker, breaking the build, or even
>> producing a non-bootable kernel in extreme cases. Typically, turning
>> on/off the lock debugging code - usually in absence of any other
>> change - may cause the following error to pop up at the link stage:
>>
>> ---
>> MODPOST vmlinux.o
>> kernel/xenomai/lock.o: In function `____xnlock_get':
>> linux/include/xenomai/cobalt/kernel/lock.h:184: undefined reference to
>> `xnlock_dbg_prepare_acquire'
>> linux/include/xenomai/cobalt/kernel/lock.h:189: undefined reference to
>> `xnlock_dbg_acquired'
>> kernel/xenomai/lock.o: In function `____xnlock_put':
>> linux/include/xenomai/cobalt/kernel/lock.h:196: undefined reference to
>> `xnlock_dbg_release'
>> ---
>>
>> Replace all occurrences of the confusing conditionals with the
>> equivalent expression explicitly testing the debug symbol.
>>
>> Signed-off-by: Philippe Gerum <rpm@xenomai.org>
>> ---
>>   include/cobalt/kernel/assert.h         |  2 +-
>>   include/cobalt/kernel/lock.h           |  6 +++---
>>   include/cobalt/kernel/sched-sporadic.h |  2 +-
>>   include/cobalt/kernel/synch.h          |  2 +-
>>   include/cobalt/kernel/tree.h           |  2 +-
>>   kernel/cobalt/bufd.c                   | 10 +++++-----
>>   kernel/cobalt/debug.c                  |  2 +-
>>   kernel/cobalt/intr.c                   |  2 +-
>>   kernel/cobalt/posix/process.c          |  2 +-
>>   kernel/cobalt/procfs.c                 |  2 +-
>>   kernel/cobalt/sched-sporadic.c         |  4 ++--
>>   kernel/cobalt/synch.c                  |  2 +-
>>   12 files changed, 19 insertions(+), 19 deletions(-)
>>
>> diff --git a/include/cobalt/kernel/assert.h
>> b/include/cobalt/kernel/assert.h
>> index 2d2d65396..86d0a480f 100644
>> --- a/include/cobalt/kernel/assert.h
>> +++ b/include/cobalt/kernel/assert.h
>> @@ -63,7 +63,7 @@
>>   #define realtime_cpu_only()    XENO_BUG_ON(CONTEXT,
>> !xnsched_supported_cpu(ipipe_processor_id()))
>>   #define thread_only()        XENO_BUG_ON(CONTEXT,
>> xnsched_interrupt_p())
>>   #define irqoff_only()        XENO_BUG_ON(CONTEXT,
>> hard_irqs_disabled() == 0)
>> -#if XENO_DEBUG(LOCKING)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>   #define atomic_only()        XENO_BUG_ON(CONTEXT,
>> (xnlock_is_owner(&nklock) && hard_irqs_disabled()) == 0)
>>   #define preemptible_only()    XENO_BUG_ON(CONTEXT,
>> xnlock_is_owner(&nklock) || hard_irqs_disabled())
>>   #else
>> diff --git a/include/cobalt/kernel/lock.h b/include/cobalt/kernel/lock.h
>> index 36f81688a..99c7848e8 100644
>> --- a/include/cobalt/kernel/lock.h
>> +++ b/include/cobalt/kernel/lock.h
>> @@ -63,7 +63,7 @@ typedef unsigned long spl_t;
>>    */
>>   #define spltest()   ipipe_test_head()
>>   -#if XENO_DEBUG(LOCKING)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>     struct xnlock {
>>       unsigned owner;
>> @@ -152,7 +152,7 @@ static inline int xnlock_dbg_release(struct xnlock
>> *lock)
>>     #endif /* !XENO_DEBUG(LOCKING) */
>>   -#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)
>> +#if defined(CONFIG_SMP) || defined(CONFIG_XENO_OPT_DEBUG_LOCKING)
>>     #define xnlock_get(lock)        __xnlock_get(lock 
>> XNLOCK_DBG_CONTEXT)
>>   #define xnlock_put(lock)        __xnlock_put(lock  XNLOCK_DBG_CONTEXT)
>> @@ -209,7 +209,7 @@ int ___xnlock_get(struct xnlock *lock /*, */
>> XNLOCK_DBG_CONTEXT_ARGS);
>>   void ___xnlock_put(struct xnlock *lock /*, */ XNLOCK_DBG_CONTEXT_ARGS);
>>   #endif /* out of line xnlock */
>>   -#if XENO_DEBUG(LOCKING)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>   /* Disable UP-over-SMP kernel optimization in debug mode. */
>>   #define __locking_active__  1
>>   #else
>> diff --git a/include/cobalt/kernel/sched-sporadic.h
>> b/include/cobalt/kernel/sched-sporadic.h
>> index 0eade47b0..50ca406c4 100644
>> --- a/include/cobalt/kernel/sched-sporadic.h
>> +++ b/include/cobalt/kernel/sched-sporadic.h
>> @@ -56,7 +56,7 @@ struct xnsched_sporadic_data {
>>   };
>>     struct xnsched_sporadic {
>> -#if XENO_DEBUG(COBALT)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>       unsigned long drop_retries;
>>   #endif
>>   };
>> diff --git a/include/cobalt/kernel/synch.h
>> b/include/cobalt/kernel/synch.h
>> index f5ac217ed..08056055a 100644
>> --- a/include/cobalt/kernel/synch.h
>> +++ b/include/cobalt/kernel/synch.h
>> @@ -101,7 +101,7 @@ static inline struct xnthread
>> *xnsynch_owner(struct xnsynch *synch)
>>   #define xnsynch_owner_check(synch, thread) \
>>       xnsynch_fast_owner_check((synch)->fastlock, thread->handle)
>>   -#if XENO_DEBUG(MUTEX_RELAXED)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>>     void xnsynch_detect_relaxed_owner(struct xnsynch *synch,
>>                     struct xnthread *sleeper);
>> diff --git a/include/cobalt/kernel/tree.h b/include/cobalt/kernel/tree.h
>> index 9c751cc02..c52ee3220 100644
>> --- a/include/cobalt/kernel/tree.h
>> +++ b/include/cobalt/kernel/tree.h
>> @@ -83,7 +83,7 @@ struct xnid *xnid_fetch(struct rb_root *t, xnkey_t key)
>>     static inline int xnid_remove(struct rb_root *t, struct xnid *xnid)
>>   {
>> -#if XENO_DEBUG(COBALT)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>       if (xnid_fetch(t, xnid->key) != xnid)
>>           return -ENOENT;
>>   #endif
>> diff --git a/kernel/cobalt/bufd.c b/kernel/cobalt/bufd.c
>> index 9b41b6ad6..3b79505d5 100644
>> --- a/kernel/cobalt/bufd.c
>> +++ b/kernel/cobalt/bufd.c
>> @@ -493,7 +493,7 @@ ssize_t xnbufd_unmap_uread(struct xnbufd *bufd)
>>   {
>>       preemptible_only();
>>   -#if XENO_DEBUG(COBALT)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>       bufd->b_ptr = (caddr_t)-1;
>>   #endif
>>       return bufd->b_off;
>> @@ -551,7 +551,7 @@ ssize_t xnbufd_unmap_uwrite(struct xnbufd *bufd)
>>       if (bufd->b_len > sizeof(bufd->b_buf))
>>           xnfree(bufd->b_carry);
>>   done:
>> -#if XENO_DEBUG(COBALT)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>       bufd->b_ptr = (caddr_t)-1;
>>   #endif
>>       return ret ?: (ssize_t)len;
>> @@ -592,7 +592,7 @@ EXPORT_SYMBOL_GPL(xnbufd_unmap_uwrite);
>>    */
>>   void xnbufd_invalidate(struct xnbufd *bufd)
>>   {
>> -#if XENO_DEBUG(COBALT)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>       bufd->b_ptr = (caddr_t)-1;
>>   #endif
>>       if (bufd->b_carry) {
>> @@ -620,7 +620,7 @@ EXPORT_SYMBOL_GPL(xnbufd_invalidate);
>>    */
>>   ssize_t xnbufd_unmap_kread(struct xnbufd *bufd)
>>   {
>> -#if XENO_DEBUG(COBALT)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>       bufd->b_ptr = (caddr_t)-1;
>>   #endif
>>       return bufd->b_off;
>> @@ -643,7 +643,7 @@ EXPORT_SYMBOL_GPL(xnbufd_unmap_kread);
>>    */
>>   ssize_t xnbufd_unmap_kwrite(struct xnbufd *bufd)
>>   {
>> -#if XENO_DEBUG(COBALT)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>       bufd->b_ptr = (caddr_t)-1;
>>   #endif
>>       return bufd->b_off;
>> diff --git a/kernel/cobalt/debug.c b/kernel/cobalt/debug.c
>> index 74022c516..af9a95ab6 100644
>> --- a/kernel/cobalt/debug.c
>> +++ b/kernel/cobalt/debug.c
>> @@ -549,7 +549,7 @@ static inline void init_thread_relax_trace(struct
>> xnthread *thread)
>>     #endif /* !XENO_OPT_DEBUG_TRACE_RELAX */
>>   -#if XENO_DEBUG(LOCKING)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>     void xnlock_dbg_prepare_acquire(unsigned long long *start)
>>   {
>> diff --git a/kernel/cobalt/intr.c b/kernel/cobalt/intr.c
>> index fb27a3295..69faa23ba 100644
>> --- a/kernel/cobalt/intr.c
>> +++ b/kernel/cobalt/intr.c
>> @@ -531,7 +531,7 @@ static inline void xnintr_irq_detach(struct xnintr
>> *intr)
>>   #else /* !CONFIG_XENO_OPT_SHIRQ */
>>     struct xnintr_vector {
>> -#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)
>> +#if defined(CONFIG_SMP) || defined(CONFIG_XENO_OPT_DEBUG_LOCKING)
>>       DECLARE_XNLOCK(lock);
>>   #endif /* CONFIG_SMP || XENO_DEBUG(LOCKING) */
>>   } ____cacheline_aligned_in_smp;
>> diff --git a/kernel/cobalt/posix/process.c
>> b/kernel/cobalt/posix/process.c
>> index dbcb81ef8..50f752f13 100644
>> --- a/kernel/cobalt/posix/process.c
>> +++ b/kernel/cobalt/posix/process.c
>> @@ -751,7 +751,7 @@ static inline int handle_exception(struct
>> ipipe_trap_data *d)
>>        * running in primary mode, move it to the Linux domain,
>>        * leaving the kernel process the exception.
>>        */
>> -#if XENO_DEBUG(COBALT) || XENO_DEBUG(USER)
>> +#if defined(CONFIG_XENO_OPT_DEBUG_COBALT) ||
>> defined(CONFIG_XENO_OPT_DEBUG_USER)
>>       if (!user_mode(d->regs)) {
>>           xntrace_panic_freeze();
>>           printk(XENO_WARNING
>> diff --git a/kernel/cobalt/procfs.c b/kernel/cobalt/procfs.c
>> index ade13cf1c..d1f01f038 100644
>> --- a/kernel/cobalt/procfs.c
>> +++ b/kernel/cobalt/procfs.c
>> @@ -27,7 +27,7 @@
>>   #include <xenomai/version.h>
>>   #include "debug.h"
>>   -#if XENO_DEBUG(LOCKING)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>     static int lock_vfile_show(struct xnvfile_regular_iterator *it,
>> void *data)
>>   {
>> diff --git a/kernel/cobalt/sched-sporadic.c
>> b/kernel/cobalt/sched-sporadic.c
>> index d2a1f76d0..9cbfa5be3 100644
>> --- a/kernel/cobalt/sched-sporadic.c
>> +++ b/kernel/cobalt/sched-sporadic.c
>> @@ -24,7 +24,7 @@
>>     static void sporadic_post_recharge(struct xnthread *thread,
>> xnticks_t budget);
>>   -#if XENO_DEBUG(COBALT)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>     static inline void sporadic_note_late_drop(struct xnsched *sched)
>>   {
>> @@ -236,7 +236,7 @@ static void xnsched_sporadic_init(struct xnsched
>> *sched)
>>        * share the same priority scale, with the addition of budget
>>        * management for the sporadic ones.
>>        */
>> -#if XENO_DEBUG(COBALT)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>       sched->pss.drop_retries = 0;
>>   #endif
>>   }
>> diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
>> index 7773a08e5..32df635a8 100644
>> --- a/kernel/cobalt/synch.c
>> +++ b/kernel/cobalt/synch.c
>> @@ -902,7 +902,7 @@ void xnsynch_release_all_ownerships(struct
>> xnthread *thread)
>>   }
>>   EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships);
>>   -#if XENO_DEBUG(MUTEX_RELAXED)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>>     /*
>>    * Detect when a thread is about to sleep on a synchronization
>>
> 
> Confused: This is not for next, is it?
> 
> E.g. that hunk above does not apply there anymore, for about a year...
> 

This is preferably aimed at the upcoming master at the moment. This does
not change any internal or external API, fixing an annoying limitation
of the watchdog. I'll soon set up my private trees, rebasing on the new
upstream branches as needed. The purpose of this patch is mainly to
trigger a review.

-- 
Philippe.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
  2018-09-05  7:44   ` Philippe Gerum
@ 2018-09-05  7:48     ` Philippe Gerum
  2018-09-05  7:57       ` Jan Kiszka
  0 siblings, 1 reply; 14+ messages in thread
From: Philippe Gerum @ 2018-09-05  7:48 UTC (permalink / raw)
  To: Jan Kiszka, xenomai

On 09/05/2018 09:44 AM, Philippe Gerum wrote:
> On 09/04/2018 07:48 PM, Jan Kiszka wrote:
>> On 2018-09-04 15:31, Philippe Gerum wrote:
>>> Testing debug switches using '#if XENO_DEBUG(<feat>)' may confuse the
>>> build system's dependency tracker, breaking the build, or even
>>> producing a non-bootable kernel in extreme cases. Typically, turning
>>> on/off the lock debugging code - usually in absence of any other
>>> change - may cause the following error to pop up at the link stage:
>>>
>>> ---
>>> MODPOST vmlinux.o
>>> kernel/xenomai/lock.o: In function `____xnlock_get':
>>> linux/include/xenomai/cobalt/kernel/lock.h:184: undefined reference to
>>> `xnlock_dbg_prepare_acquire'
>>> linux/include/xenomai/cobalt/kernel/lock.h:189: undefined reference to
>>> `xnlock_dbg_acquired'
>>> kernel/xenomai/lock.o: In function `____xnlock_put':
>>> linux/include/xenomai/cobalt/kernel/lock.h:196: undefined reference to
>>> `xnlock_dbg_release'
>>> ---
>>>
>>> Replace all occurrences of the confusing conditionals with the
>>> equivalent expression explicitly testing the debug symbol.
>>>
>>> Signed-off-by: Philippe Gerum <rpm@xenomai.org>
>>> ---
>>>   include/cobalt/kernel/assert.h         |  2 +-
>>>   include/cobalt/kernel/lock.h           |  6 +++---
>>>   include/cobalt/kernel/sched-sporadic.h |  2 +-
>>>   include/cobalt/kernel/synch.h          |  2 +-
>>>   include/cobalt/kernel/tree.h           |  2 +-
>>>   kernel/cobalt/bufd.c                   | 10 +++++-----
>>>   kernel/cobalt/debug.c                  |  2 +-
>>>   kernel/cobalt/intr.c                   |  2 +-
>>>   kernel/cobalt/posix/process.c          |  2 +-
>>>   kernel/cobalt/procfs.c                 |  2 +-
>>>   kernel/cobalt/sched-sporadic.c         |  4 ++--
>>>   kernel/cobalt/synch.c                  |  2 +-
>>>   12 files changed, 19 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/include/cobalt/kernel/assert.h
>>> b/include/cobalt/kernel/assert.h
>>> index 2d2d65396..86d0a480f 100644
>>> --- a/include/cobalt/kernel/assert.h
>>> +++ b/include/cobalt/kernel/assert.h
>>> @@ -63,7 +63,7 @@
>>>   #define realtime_cpu_only()    XENO_BUG_ON(CONTEXT,
>>> !xnsched_supported_cpu(ipipe_processor_id()))
>>>   #define thread_only()        XENO_BUG_ON(CONTEXT,
>>> xnsched_interrupt_p())
>>>   #define irqoff_only()        XENO_BUG_ON(CONTEXT,
>>> hard_irqs_disabled() == 0)
>>> -#if XENO_DEBUG(LOCKING)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>>   #define atomic_only()        XENO_BUG_ON(CONTEXT,
>>> (xnlock_is_owner(&nklock) && hard_irqs_disabled()) == 0)
>>>   #define preemptible_only()    XENO_BUG_ON(CONTEXT,
>>> xnlock_is_owner(&nklock) || hard_irqs_disabled())
>>>   #else
>>> diff --git a/include/cobalt/kernel/lock.h b/include/cobalt/kernel/lock.h
>>> index 36f81688a..99c7848e8 100644
>>> --- a/include/cobalt/kernel/lock.h
>>> +++ b/include/cobalt/kernel/lock.h
>>> @@ -63,7 +63,7 @@ typedef unsigned long spl_t;
>>>    */
>>>   #define spltest()   ipipe_test_head()
>>>   -#if XENO_DEBUG(LOCKING)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>>     struct xnlock {
>>>       unsigned owner;
>>> @@ -152,7 +152,7 @@ static inline int xnlock_dbg_release(struct xnlock
>>> *lock)
>>>     #endif /* !XENO_DEBUG(LOCKING) */
>>>   -#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)
>>> +#if defined(CONFIG_SMP) || defined(CONFIG_XENO_OPT_DEBUG_LOCKING)
>>>     #define xnlock_get(lock)        __xnlock_get(lock 
>>> XNLOCK_DBG_CONTEXT)
>>>   #define xnlock_put(lock)        __xnlock_put(lock  XNLOCK_DBG_CONTEXT)
>>> @@ -209,7 +209,7 @@ int ___xnlock_get(struct xnlock *lock /*, */
>>> XNLOCK_DBG_CONTEXT_ARGS);
>>>   void ___xnlock_put(struct xnlock *lock /*, */ XNLOCK_DBG_CONTEXT_ARGS);
>>>   #endif /* out of line xnlock */
>>>   -#if XENO_DEBUG(LOCKING)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>>   /* Disable UP-over-SMP kernel optimization in debug mode. */
>>>   #define __locking_active__  1
>>>   #else
>>> diff --git a/include/cobalt/kernel/sched-sporadic.h
>>> b/include/cobalt/kernel/sched-sporadic.h
>>> index 0eade47b0..50ca406c4 100644
>>> --- a/include/cobalt/kernel/sched-sporadic.h
>>> +++ b/include/cobalt/kernel/sched-sporadic.h
>>> @@ -56,7 +56,7 @@ struct xnsched_sporadic_data {
>>>   };
>>>     struct xnsched_sporadic {
>>> -#if XENO_DEBUG(COBALT)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>       unsigned long drop_retries;
>>>   #endif
>>>   };
>>> diff --git a/include/cobalt/kernel/synch.h
>>> b/include/cobalt/kernel/synch.h
>>> index f5ac217ed..08056055a 100644
>>> --- a/include/cobalt/kernel/synch.h
>>> +++ b/include/cobalt/kernel/synch.h
>>> @@ -101,7 +101,7 @@ static inline struct xnthread
>>> *xnsynch_owner(struct xnsynch *synch)
>>>   #define xnsynch_owner_check(synch, thread) \
>>>       xnsynch_fast_owner_check((synch)->fastlock, thread->handle)
>>>   -#if XENO_DEBUG(MUTEX_RELAXED)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>>>     void xnsynch_detect_relaxed_owner(struct xnsynch *synch,
>>>                     struct xnthread *sleeper);
>>> diff --git a/include/cobalt/kernel/tree.h b/include/cobalt/kernel/tree.h
>>> index 9c751cc02..c52ee3220 100644
>>> --- a/include/cobalt/kernel/tree.h
>>> +++ b/include/cobalt/kernel/tree.h
>>> @@ -83,7 +83,7 @@ struct xnid *xnid_fetch(struct rb_root *t, xnkey_t key)
>>>     static inline int xnid_remove(struct rb_root *t, struct xnid *xnid)
>>>   {
>>> -#if XENO_DEBUG(COBALT)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>       if (xnid_fetch(t, xnid->key) != xnid)
>>>           return -ENOENT;
>>>   #endif
>>> diff --git a/kernel/cobalt/bufd.c b/kernel/cobalt/bufd.c
>>> index 9b41b6ad6..3b79505d5 100644
>>> --- a/kernel/cobalt/bufd.c
>>> +++ b/kernel/cobalt/bufd.c
>>> @@ -493,7 +493,7 @@ ssize_t xnbufd_unmap_uread(struct xnbufd *bufd)
>>>   {
>>>       preemptible_only();
>>>   -#if XENO_DEBUG(COBALT)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>       bufd->b_ptr = (caddr_t)-1;
>>>   #endif
>>>       return bufd->b_off;
>>> @@ -551,7 +551,7 @@ ssize_t xnbufd_unmap_uwrite(struct xnbufd *bufd)
>>>       if (bufd->b_len > sizeof(bufd->b_buf))
>>>           xnfree(bufd->b_carry);
>>>   done:
>>> -#if XENO_DEBUG(COBALT)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>       bufd->b_ptr = (caddr_t)-1;
>>>   #endif
>>>       return ret ?: (ssize_t)len;
>>> @@ -592,7 +592,7 @@ EXPORT_SYMBOL_GPL(xnbufd_unmap_uwrite);
>>>    */
>>>   void xnbufd_invalidate(struct xnbufd *bufd)
>>>   {
>>> -#if XENO_DEBUG(COBALT)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>       bufd->b_ptr = (caddr_t)-1;
>>>   #endif
>>>       if (bufd->b_carry) {
>>> @@ -620,7 +620,7 @@ EXPORT_SYMBOL_GPL(xnbufd_invalidate);
>>>    */
>>>   ssize_t xnbufd_unmap_kread(struct xnbufd *bufd)
>>>   {
>>> -#if XENO_DEBUG(COBALT)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>       bufd->b_ptr = (caddr_t)-1;
>>>   #endif
>>>       return bufd->b_off;
>>> @@ -643,7 +643,7 @@ EXPORT_SYMBOL_GPL(xnbufd_unmap_kread);
>>>    */
>>>   ssize_t xnbufd_unmap_kwrite(struct xnbufd *bufd)
>>>   {
>>> -#if XENO_DEBUG(COBALT)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>       bufd->b_ptr = (caddr_t)-1;
>>>   #endif
>>>       return bufd->b_off;
>>> diff --git a/kernel/cobalt/debug.c b/kernel/cobalt/debug.c
>>> index 74022c516..af9a95ab6 100644
>>> --- a/kernel/cobalt/debug.c
>>> +++ b/kernel/cobalt/debug.c
>>> @@ -549,7 +549,7 @@ static inline void init_thread_relax_trace(struct
>>> xnthread *thread)
>>>     #endif /* !XENO_OPT_DEBUG_TRACE_RELAX */
>>>   -#if XENO_DEBUG(LOCKING)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>>     void xnlock_dbg_prepare_acquire(unsigned long long *start)
>>>   {
>>> diff --git a/kernel/cobalt/intr.c b/kernel/cobalt/intr.c
>>> index fb27a3295..69faa23ba 100644
>>> --- a/kernel/cobalt/intr.c
>>> +++ b/kernel/cobalt/intr.c
>>> @@ -531,7 +531,7 @@ static inline void xnintr_irq_detach(struct xnintr
>>> *intr)
>>>   #else /* !CONFIG_XENO_OPT_SHIRQ */
>>>     struct xnintr_vector {
>>> -#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)
>>> +#if defined(CONFIG_SMP) || defined(CONFIG_XENO_OPT_DEBUG_LOCKING)
>>>       DECLARE_XNLOCK(lock);
>>>   #endif /* CONFIG_SMP || XENO_DEBUG(LOCKING) */
>>>   } ____cacheline_aligned_in_smp;
>>> diff --git a/kernel/cobalt/posix/process.c
>>> b/kernel/cobalt/posix/process.c
>>> index dbcb81ef8..50f752f13 100644
>>> --- a/kernel/cobalt/posix/process.c
>>> +++ b/kernel/cobalt/posix/process.c
>>> @@ -751,7 +751,7 @@ static inline int handle_exception(struct
>>> ipipe_trap_data *d)
>>>        * running in primary mode, move it to the Linux domain,
>>>        * leaving the kernel process the exception.
>>>        */
>>> -#if XENO_DEBUG(COBALT) || XENO_DEBUG(USER)
>>> +#if defined(CONFIG_XENO_OPT_DEBUG_COBALT) ||
>>> defined(CONFIG_XENO_OPT_DEBUG_USER)
>>>       if (!user_mode(d->regs)) {
>>>           xntrace_panic_freeze();
>>>           printk(XENO_WARNING
>>> diff --git a/kernel/cobalt/procfs.c b/kernel/cobalt/procfs.c
>>> index ade13cf1c..d1f01f038 100644
>>> --- a/kernel/cobalt/procfs.c
>>> +++ b/kernel/cobalt/procfs.c
>>> @@ -27,7 +27,7 @@
>>>   #include <xenomai/version.h>
>>>   #include "debug.h"
>>>   -#if XENO_DEBUG(LOCKING)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>>     static int lock_vfile_show(struct xnvfile_regular_iterator *it,
>>> void *data)
>>>   {
>>> diff --git a/kernel/cobalt/sched-sporadic.c
>>> b/kernel/cobalt/sched-sporadic.c
>>> index d2a1f76d0..9cbfa5be3 100644
>>> --- a/kernel/cobalt/sched-sporadic.c
>>> +++ b/kernel/cobalt/sched-sporadic.c
>>> @@ -24,7 +24,7 @@
>>>     static void sporadic_post_recharge(struct xnthread *thread,
>>> xnticks_t budget);
>>>   -#if XENO_DEBUG(COBALT)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>     static inline void sporadic_note_late_drop(struct xnsched *sched)
>>>   {
>>> @@ -236,7 +236,7 @@ static void xnsched_sporadic_init(struct xnsched
>>> *sched)
>>>        * share the same priority scale, with the addition of budget
>>>        * management for the sporadic ones.
>>>        */
>>> -#if XENO_DEBUG(COBALT)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>       sched->pss.drop_retries = 0;
>>>   #endif
>>>   }
>>> diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
>>> index 7773a08e5..32df635a8 100644
>>> --- a/kernel/cobalt/synch.c
>>> +++ b/kernel/cobalt/synch.c
>>> @@ -902,7 +902,7 @@ void xnsynch_release_all_ownerships(struct
>>> xnthread *thread)
>>>   }
>>>   EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships);
>>>   -#if XENO_DEBUG(MUTEX_RELAXED)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>>>     /*
>>>    * Detect when a thread is about to sleep on a synchronization
>>>
>>
>> Confused: This is not for next, is it?
>>
>> E.g. that hunk above does not apply there anymore, for about a year...
>>
> 
> This is preferably aimed at the upcoming master at the moment. This does
> not change any internal or external API, fixing an annoying limitation
> of the watchdog.

The quite annoying issue is about generating bad kernels in this case,
same reasoning with the watchdog issue.

-- 
Philippe.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
  2018-09-05  7:48     ` Philippe Gerum
@ 2018-09-05  7:57       ` Jan Kiszka
  0 siblings, 0 replies; 14+ messages in thread
From: Jan Kiszka @ 2018-09-05  7:57 UTC (permalink / raw)
  To: Philippe Gerum, xenomai

On 2018-09-05 09:48, Philippe Gerum wrote:
> On 09/05/2018 09:44 AM, Philippe Gerum wrote:
>> On 09/04/2018 07:48 PM, Jan Kiszka wrote:
>>> On 2018-09-04 15:31, Philippe Gerum wrote:
>>>> Testing debug switches using '#if XENO_DEBUG(<feat>)' may confuse the
>>>> build system's dependency tracker, breaking the build, or even
>>>> producing a non-bootable kernel in extreme cases. Typically, turning
>>>> on/off the lock debugging code - usually in absence of any other
>>>> change - may cause the following error to pop up at the link stage:
>>>>
>>>> ---
>>>> MODPOST vmlinux.o
>>>> kernel/xenomai/lock.o: In function `____xnlock_get':
>>>> linux/include/xenomai/cobalt/kernel/lock.h:184: undefined reference to
>>>> `xnlock_dbg_prepare_acquire'
>>>> linux/include/xenomai/cobalt/kernel/lock.h:189: undefined reference to
>>>> `xnlock_dbg_acquired'
>>>> kernel/xenomai/lock.o: In function `____xnlock_put':
>>>> linux/include/xenomai/cobalt/kernel/lock.h:196: undefined reference to
>>>> `xnlock_dbg_release'
>>>> ---
>>>>
>>>> Replace all occurrences of the confusing conditionals with the
>>>> equivalent expression explicitly testing the debug symbol.
>>>>
>>>> Signed-off-by: Philippe Gerum <rpm@xenomai.org>
>>>> ---
>>>>    include/cobalt/kernel/assert.h         |  2 +-
>>>>    include/cobalt/kernel/lock.h           |  6 +++---
>>>>    include/cobalt/kernel/sched-sporadic.h |  2 +-
>>>>    include/cobalt/kernel/synch.h          |  2 +-
>>>>    include/cobalt/kernel/tree.h           |  2 +-
>>>>    kernel/cobalt/bufd.c                   | 10 +++++-----
>>>>    kernel/cobalt/debug.c                  |  2 +-
>>>>    kernel/cobalt/intr.c                   |  2 +-
>>>>    kernel/cobalt/posix/process.c          |  2 +-
>>>>    kernel/cobalt/procfs.c                 |  2 +-
>>>>    kernel/cobalt/sched-sporadic.c         |  4 ++--
>>>>    kernel/cobalt/synch.c                  |  2 +-
>>>>    12 files changed, 19 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/include/cobalt/kernel/assert.h
>>>> b/include/cobalt/kernel/assert.h
>>>> index 2d2d65396..86d0a480f 100644
>>>> --- a/include/cobalt/kernel/assert.h
>>>> +++ b/include/cobalt/kernel/assert.h
>>>> @@ -63,7 +63,7 @@
>>>>    #define realtime_cpu_only()    XENO_BUG_ON(CONTEXT,
>>>> !xnsched_supported_cpu(ipipe_processor_id()))
>>>>    #define thread_only()        XENO_BUG_ON(CONTEXT,
>>>> xnsched_interrupt_p())
>>>>    #define irqoff_only()        XENO_BUG_ON(CONTEXT,
>>>> hard_irqs_disabled() == 0)
>>>> -#if XENO_DEBUG(LOCKING)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>>>    #define atomic_only()        XENO_BUG_ON(CONTEXT,
>>>> (xnlock_is_owner(&nklock) && hard_irqs_disabled()) == 0)
>>>>    #define preemptible_only()    XENO_BUG_ON(CONTEXT,
>>>> xnlock_is_owner(&nklock) || hard_irqs_disabled())
>>>>    #else
>>>> diff --git a/include/cobalt/kernel/lock.h b/include/cobalt/kernel/lock.h
>>>> index 36f81688a..99c7848e8 100644
>>>> --- a/include/cobalt/kernel/lock.h
>>>> +++ b/include/cobalt/kernel/lock.h
>>>> @@ -63,7 +63,7 @@ typedef unsigned long spl_t;
>>>>     */
>>>>    #define spltest()   ipipe_test_head()
>>>>    -#if XENO_DEBUG(LOCKING)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>>>      struct xnlock {
>>>>        unsigned owner;
>>>> @@ -152,7 +152,7 @@ static inline int xnlock_dbg_release(struct xnlock
>>>> *lock)
>>>>      #endif /* !XENO_DEBUG(LOCKING) */
>>>>    -#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)
>>>> +#if defined(CONFIG_SMP) || defined(CONFIG_XENO_OPT_DEBUG_LOCKING)
>>>>      #define xnlock_get(lock)        __xnlock_get(lock
>>>> XNLOCK_DBG_CONTEXT)
>>>>    #define xnlock_put(lock)        __xnlock_put(lock  XNLOCK_DBG_CONTEXT)
>>>> @@ -209,7 +209,7 @@ int ___xnlock_get(struct xnlock *lock /*, */
>>>> XNLOCK_DBG_CONTEXT_ARGS);
>>>>    void ___xnlock_put(struct xnlock *lock /*, */ XNLOCK_DBG_CONTEXT_ARGS);
>>>>    #endif /* out of line xnlock */
>>>>    -#if XENO_DEBUG(LOCKING)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>>>    /* Disable UP-over-SMP kernel optimization in debug mode. */
>>>>    #define __locking_active__  1
>>>>    #else
>>>> diff --git a/include/cobalt/kernel/sched-sporadic.h
>>>> b/include/cobalt/kernel/sched-sporadic.h
>>>> index 0eade47b0..50ca406c4 100644
>>>> --- a/include/cobalt/kernel/sched-sporadic.h
>>>> +++ b/include/cobalt/kernel/sched-sporadic.h
>>>> @@ -56,7 +56,7 @@ struct xnsched_sporadic_data {
>>>>    };
>>>>      struct xnsched_sporadic {
>>>> -#if XENO_DEBUG(COBALT)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>>        unsigned long drop_retries;
>>>>    #endif
>>>>    };
>>>> diff --git a/include/cobalt/kernel/synch.h
>>>> b/include/cobalt/kernel/synch.h
>>>> index f5ac217ed..08056055a 100644
>>>> --- a/include/cobalt/kernel/synch.h
>>>> +++ b/include/cobalt/kernel/synch.h
>>>> @@ -101,7 +101,7 @@ static inline struct xnthread
>>>> *xnsynch_owner(struct xnsynch *synch)
>>>>    #define xnsynch_owner_check(synch, thread) \
>>>>        xnsynch_fast_owner_check((synch)->fastlock, thread->handle)
>>>>    -#if XENO_DEBUG(MUTEX_RELAXED)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>>>>      void xnsynch_detect_relaxed_owner(struct xnsynch *synch,
>>>>                      struct xnthread *sleeper);
>>>> diff --git a/include/cobalt/kernel/tree.h b/include/cobalt/kernel/tree.h
>>>> index 9c751cc02..c52ee3220 100644
>>>> --- a/include/cobalt/kernel/tree.h
>>>> +++ b/include/cobalt/kernel/tree.h
>>>> @@ -83,7 +83,7 @@ struct xnid *xnid_fetch(struct rb_root *t, xnkey_t key)
>>>>      static inline int xnid_remove(struct rb_root *t, struct xnid *xnid)
>>>>    {
>>>> -#if XENO_DEBUG(COBALT)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>>        if (xnid_fetch(t, xnid->key) != xnid)
>>>>            return -ENOENT;
>>>>    #endif
>>>> diff --git a/kernel/cobalt/bufd.c b/kernel/cobalt/bufd.c
>>>> index 9b41b6ad6..3b79505d5 100644
>>>> --- a/kernel/cobalt/bufd.c
>>>> +++ b/kernel/cobalt/bufd.c
>>>> @@ -493,7 +493,7 @@ ssize_t xnbufd_unmap_uread(struct xnbufd *bufd)
>>>>    {
>>>>        preemptible_only();
>>>>    -#if XENO_DEBUG(COBALT)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>>        bufd->b_ptr = (caddr_t)-1;
>>>>    #endif
>>>>        return bufd->b_off;
>>>> @@ -551,7 +551,7 @@ ssize_t xnbufd_unmap_uwrite(struct xnbufd *bufd)
>>>>        if (bufd->b_len > sizeof(bufd->b_buf))
>>>>            xnfree(bufd->b_carry);
>>>>    done:
>>>> -#if XENO_DEBUG(COBALT)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>>        bufd->b_ptr = (caddr_t)-1;
>>>>    #endif
>>>>        return ret ?: (ssize_t)len;
>>>> @@ -592,7 +592,7 @@ EXPORT_SYMBOL_GPL(xnbufd_unmap_uwrite);
>>>>     */
>>>>    void xnbufd_invalidate(struct xnbufd *bufd)
>>>>    {
>>>> -#if XENO_DEBUG(COBALT)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>>        bufd->b_ptr = (caddr_t)-1;
>>>>    #endif
>>>>        if (bufd->b_carry) {
>>>> @@ -620,7 +620,7 @@ EXPORT_SYMBOL_GPL(xnbufd_invalidate);
>>>>     */
>>>>    ssize_t xnbufd_unmap_kread(struct xnbufd *bufd)
>>>>    {
>>>> -#if XENO_DEBUG(COBALT)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>>        bufd->b_ptr = (caddr_t)-1;
>>>>    #endif
>>>>        return bufd->b_off;
>>>> @@ -643,7 +643,7 @@ EXPORT_SYMBOL_GPL(xnbufd_unmap_kread);
>>>>     */
>>>>    ssize_t xnbufd_unmap_kwrite(struct xnbufd *bufd)
>>>>    {
>>>> -#if XENO_DEBUG(COBALT)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>>        bufd->b_ptr = (caddr_t)-1;
>>>>    #endif
>>>>        return bufd->b_off;
>>>> diff --git a/kernel/cobalt/debug.c b/kernel/cobalt/debug.c
>>>> index 74022c516..af9a95ab6 100644
>>>> --- a/kernel/cobalt/debug.c
>>>> +++ b/kernel/cobalt/debug.c
>>>> @@ -549,7 +549,7 @@ static inline void init_thread_relax_trace(struct
>>>> xnthread *thread)
>>>>      #endif /* !XENO_OPT_DEBUG_TRACE_RELAX */
>>>>    -#if XENO_DEBUG(LOCKING)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>>>      void xnlock_dbg_prepare_acquire(unsigned long long *start)
>>>>    {
>>>> diff --git a/kernel/cobalt/intr.c b/kernel/cobalt/intr.c
>>>> index fb27a3295..69faa23ba 100644
>>>> --- a/kernel/cobalt/intr.c
>>>> +++ b/kernel/cobalt/intr.c
>>>> @@ -531,7 +531,7 @@ static inline void xnintr_irq_detach(struct xnintr
>>>> *intr)
>>>>    #else /* !CONFIG_XENO_OPT_SHIRQ */
>>>>      struct xnintr_vector {
>>>> -#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)
>>>> +#if defined(CONFIG_SMP) || defined(CONFIG_XENO_OPT_DEBUG_LOCKING)
>>>>        DECLARE_XNLOCK(lock);
>>>>    #endif /* CONFIG_SMP || XENO_DEBUG(LOCKING) */
>>>>    } ____cacheline_aligned_in_smp;
>>>> diff --git a/kernel/cobalt/posix/process.c
>>>> b/kernel/cobalt/posix/process.c
>>>> index dbcb81ef8..50f752f13 100644
>>>> --- a/kernel/cobalt/posix/process.c
>>>> +++ b/kernel/cobalt/posix/process.c
>>>> @@ -751,7 +751,7 @@ static inline int handle_exception(struct
>>>> ipipe_trap_data *d)
>>>>         * running in primary mode, move it to the Linux domain,
>>>>         * leaving the kernel process the exception.
>>>>         */
>>>> -#if XENO_DEBUG(COBALT) || XENO_DEBUG(USER)
>>>> +#if defined(CONFIG_XENO_OPT_DEBUG_COBALT) ||
>>>> defined(CONFIG_XENO_OPT_DEBUG_USER)
>>>>        if (!user_mode(d->regs)) {
>>>>            xntrace_panic_freeze();
>>>>            printk(XENO_WARNING
>>>> diff --git a/kernel/cobalt/procfs.c b/kernel/cobalt/procfs.c
>>>> index ade13cf1c..d1f01f038 100644
>>>> --- a/kernel/cobalt/procfs.c
>>>> +++ b/kernel/cobalt/procfs.c
>>>> @@ -27,7 +27,7 @@
>>>>    #include <xenomai/version.h>
>>>>    #include "debug.h"
>>>>    -#if XENO_DEBUG(LOCKING)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>>>>      static int lock_vfile_show(struct xnvfile_regular_iterator *it,
>>>> void *data)
>>>>    {
>>>> diff --git a/kernel/cobalt/sched-sporadic.c
>>>> b/kernel/cobalt/sched-sporadic.c
>>>> index d2a1f76d0..9cbfa5be3 100644
>>>> --- a/kernel/cobalt/sched-sporadic.c
>>>> +++ b/kernel/cobalt/sched-sporadic.c
>>>> @@ -24,7 +24,7 @@
>>>>      static void sporadic_post_recharge(struct xnthread *thread,
>>>> xnticks_t budget);
>>>>    -#if XENO_DEBUG(COBALT)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>>      static inline void sporadic_note_late_drop(struct xnsched *sched)
>>>>    {
>>>> @@ -236,7 +236,7 @@ static void xnsched_sporadic_init(struct xnsched
>>>> *sched)
>>>>         * share the same priority scale, with the addition of budget
>>>>         * management for the sporadic ones.
>>>>         */
>>>> -#if XENO_DEBUG(COBALT)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>>>>        sched->pss.drop_retries = 0;
>>>>    #endif
>>>>    }
>>>> diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
>>>> index 7773a08e5..32df635a8 100644
>>>> --- a/kernel/cobalt/synch.c
>>>> +++ b/kernel/cobalt/synch.c
>>>> @@ -902,7 +902,7 @@ void xnsynch_release_all_ownerships(struct
>>>> xnthread *thread)
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships);
>>>>    -#if XENO_DEBUG(MUTEX_RELAXED)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>>>>      /*
>>>>     * Detect when a thread is about to sleep on a synchronization
>>>>
>>>
>>> Confused: This is not for next, is it?
>>>
>>> E.g. that hunk above does not apply there anymore, for about a year...
>>>
>>
>> This is preferably aimed at the upcoming master at the moment. This does
>> not change any internal or external API, fixing an annoying limitation
>> of the watchdog.
> 
> The quite annoying issue is about generating bad kernels in this case,
> same reasoning with the watchdog issue.
> 

As it's based on a seriously old code base, I can't really review this 
patch. Please rebase everything first.

Upcoming master will simply be current next + X as soon as we agree on 
the workflow I proposed. If that does not work via fast-forward merge, 
we may need a hard reset once. I'll look into that later today.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
  2018-09-04 17:48 ` Jan Kiszka
  2018-09-05  7:44   ` Philippe Gerum
@ 2018-09-05  8:16   ` Philippe Gerum
  2018-09-05  8:34     ` Jan Kiszka
  1 sibling, 1 reply; 14+ messages in thread
From: Philippe Gerum @ 2018-09-05  8:16 UTC (permalink / raw)
  To: Jan Kiszka, xenomai

On 09/04/2018 07:48 PM, Jan Kiszka wrote:
>> diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
>> index 7773a08e5..32df635a8 100644
>> --- a/kernel/cobalt/synch.c
>> +++ b/kernel/cobalt/synch.c
>> @@ -902,7 +902,7 @@ void xnsynch_release_all_ownerships(struct
>> xnthread *thread)
>>   }
>>   EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships);
>>   -#if XENO_DEBUG(MUTEX_RELAXED)
>> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>>     /*
>>    * Detect when a thread is about to sleep on a synchronization
>>
> 
> Confused: This is not for next, is it?
> 
> E.g. that hunk above does not apply there anymore, for about a year...
> 

Something looks wrong here. The three patches apply on commit #e23d58715
[1] at [2], which is on top of stable/v3.0.x, post-3.0.7 material. Which
tree/branch are you looking at?


[1]
commit e23d587156f138a8f9bff648e310427127a98cfc (origin/stable/v3.0.x)
Author: Sebastian Smolorz <sebastian.smolorz@gmx.de>
Date:   Thu Aug 16 21:10:00 2018 +0200

    net: Split internal rtnet.h and install rtdm/net.h and rtdm/uapi/net.h

    For external projects wanting to use RTnet with its special
    features like RTnet IOCTLs it is necessary to provide the API
    headers in the Xenomai installation directory.

[2] https://gitlab.denx.de/Xenomai/xenomai/commits/stable/v3.0.x

-- 
Philippe.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
  2018-09-05  8:16   ` Philippe Gerum
@ 2018-09-05  8:34     ` Jan Kiszka
  2018-09-05  9:33       ` Philippe Gerum
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2018-09-05  8:34 UTC (permalink / raw)
  To: Philippe Gerum, xenomai

On 2018-09-05 10:16, Philippe Gerum wrote:
> On 09/04/2018 07:48 PM, Jan Kiszka wrote:
>>> diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
>>> index 7773a08e5..32df635a8 100644
>>> --- a/kernel/cobalt/synch.c
>>> +++ b/kernel/cobalt/synch.c
>>> @@ -902,7 +902,7 @@ void xnsynch_release_all_ownerships(struct
>>> xnthread *thread)
>>>    }
>>>    EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships);
>>>    -#if XENO_DEBUG(MUTEX_RELAXED)
>>> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>>>      /*
>>>     * Detect when a thread is about to sleep on a synchronization
>>>
>>
>> Confused: This is not for next, is it?
>>
>> E.g. that hunk above does not apply there anymore, for about a year...
>>
> 
> Something looks wrong here. The three patches apply on commit #e23d58715
> [1] at [2], which is on top of stable/v3.0.x, post-3.0.7 material. Which
> tree/branch are you looking at?

next - stable is yours.

If you like review of stable backports, please tag those commits as 
[stable] or so. And maybe also explain any deviation.

My vision would be that only patches that are in next/master would make 
it to stable. Ideally, no or only minor tweaking should be needed to 
such commits for the backport.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
  2018-09-05  8:34     ` Jan Kiszka
@ 2018-09-05  9:33       ` Philippe Gerum
  2018-09-05  9:44         ` Jan Kiszka
  0 siblings, 1 reply; 14+ messages in thread
From: Philippe Gerum @ 2018-09-05  9:33 UTC (permalink / raw)
  To: Jan Kiszka, xenomai

On 09/05/2018 10:34 AM, Jan Kiszka wrote:
> On 2018-09-05 10:16, Philippe Gerum wrote:
>> On 09/04/2018 07:48 PM, Jan Kiszka wrote:
>>>> diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
>>>> index 7773a08e5..32df635a8 100644
>>>> --- a/kernel/cobalt/synch.c
>>>> +++ b/kernel/cobalt/synch.c
>>>> @@ -902,7 +902,7 @@ void xnsynch_release_all_ownerships(struct
>>>> xnthread *thread)
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships);
>>>>    -#if XENO_DEBUG(MUTEX_RELAXED)
>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>>>>      /*
>>>>     * Detect when a thread is about to sleep on a synchronization
>>>>
>>>
>>> Confused: This is not for next, is it?
>>>
>>> E.g. that hunk above does not apply there anymore, for about a year...
>>>
>>
>> Something looks wrong here. The three patches apply on commit #e23d58715
>> [1] at [2], which is on top of stable/v3.0.x, post-3.0.7 material. Which
>> tree/branch are you looking at?
> 
> next - stable is yours.
> 

Is it? Ok, for now, until a -stable maintainer is appointed eventually.

> If you like review of stable backports, please tag those commits as
> [stable] or so. And maybe also explain any deviation.
> 

The way -stable was used is obviously different in the new process, and
the patches were consistent with the previous process, so let's drop
those patches if you can't review them, I'll rebase over the upcoming
-next branch to make it easier for everyone.

> My vision would be that only patches that are in next/master would make
> it to stable.

I understand that you want to align with the basics of the mainline
kernel workflow, I'm ok with this. Please set (or re-purpose) the
(existing) git branches on gitlab the way you want to, to clarify the
situation.

 Ideally, no or only minor tweaking should be needed to
> such commits for the backport.
> 

This should be ok in most cases. Unfortunately, the XENO_DEBUG() fixes
are going to be an exception because they have to spread all over the
map - unless they don't go to stable, which could be an issue since the
corresponding bug may produce bad kernels.

-- 
Philippe.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
  2018-09-05  9:33       ` Philippe Gerum
@ 2018-09-05  9:44         ` Jan Kiszka
  2018-09-05 10:22           ` Philippe Gerum
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2018-09-05  9:44 UTC (permalink / raw)
  To: Philippe Gerum, xenomai

On 2018-09-05 11:33, Philippe Gerum wrote:
> On 09/05/2018 10:34 AM, Jan Kiszka wrote:
>> On 2018-09-05 10:16, Philippe Gerum wrote:
>>> On 09/04/2018 07:48 PM, Jan Kiszka wrote:
>>>>> diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
>>>>> index 7773a08e5..32df635a8 100644
>>>>> --- a/kernel/cobalt/synch.c
>>>>> +++ b/kernel/cobalt/synch.c
>>>>> @@ -902,7 +902,7 @@ void xnsynch_release_all_ownerships(struct
>>>>> xnthread *thread)
>>>>>     }
>>>>>     EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships);
>>>>>     -#if XENO_DEBUG(MUTEX_RELAXED)
>>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>>>>>       /*
>>>>>      * Detect when a thread is about to sleep on a synchronization
>>>>>
>>>>
>>>> Confused: This is not for next, is it?
>>>>
>>>> E.g. that hunk above does not apply there anymore, for about a year...
>>>>
>>>
>>> Something looks wrong here. The three patches apply on commit #e23d58715
>>> [1] at [2], which is on top of stable/v3.0.x, post-3.0.7 material. Which
>>> tree/branch are you looking at?
>>
>> next - stable is yours.
>>
> 
> Is it? Ok, for now, until a -stable maintainer is appointed eventually.
> 
>> If you like review of stable backports, please tag those commits as
>> [stable] or so. And maybe also explain any deviation.
>>
> 
> The way -stable was used is obviously different in the new process, and
> the patches were consistent with the previous process, so let's drop
> those patches if you can't review them, I'll rebase over the upcoming
> -next branch to make it easier for everyone.
> 
>> My vision would be that only patches that are in next/master would make
>> it to stable.
> 
> I understand that you want to align with the basics of the mainline
> kernel workflow, I'm ok with this. Please set (or re-purpose) the
> (existing) git branches on gitlab the way you want to, to clarify the
> situation.
> 
>   Ideally, no or only minor tweaking should be needed to
>> such commits for the backport.
>>
> 
> This should be ok in most cases. Unfortunately, the XENO_DEBUG() fixes
> are going to be an exception because they have to spread all over the
> map - unless they don't go to stable, which could be an issue since the
> corresponding bug may produce bad kernels.
> 

It seems we addressed most of the XENO_DEBUG cases in next. Are the 
related commits no stable candidates because they do more? If they are, 
I would start with backporting them, and then address what remains.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
  2018-09-05  9:44         ` Jan Kiszka
@ 2018-09-05 10:22           ` Philippe Gerum
  0 siblings, 0 replies; 14+ messages in thread
From: Philippe Gerum @ 2018-09-05 10:22 UTC (permalink / raw)
  To: Jan Kiszka, xenomai

On 09/05/2018 11:44 AM, Jan Kiszka wrote:
> On 2018-09-05 11:33, Philippe Gerum wrote:
>> On 09/05/2018 10:34 AM, Jan Kiszka wrote:
>>> On 2018-09-05 10:16, Philippe Gerum wrote:
>>>> On 09/04/2018 07:48 PM, Jan Kiszka wrote:
>>>>>> diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
>>>>>> index 7773a08e5..32df635a8 100644
>>>>>> --- a/kernel/cobalt/synch.c
>>>>>> +++ b/kernel/cobalt/synch.c
>>>>>> @@ -902,7 +902,7 @@ void xnsynch_release_all_ownerships(struct
>>>>>> xnthread *thread)
>>>>>>     }
>>>>>>     EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships);
>>>>>>     -#if XENO_DEBUG(MUTEX_RELAXED)
>>>>>> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>>>>>>       /*
>>>>>>      * Detect when a thread is about to sleep on a synchronization
>>>>>>
>>>>>
>>>>> Confused: This is not for next, is it?
>>>>>
>>>>> E.g. that hunk above does not apply there anymore, for about a year...
>>>>>
>>>>
>>>> Something looks wrong here. The three patches apply on commit
>>>> #e23d58715
>>>> [1] at [2], which is on top of stable/v3.0.x, post-3.0.7 material.
>>>> Which
>>>> tree/branch are you looking at?
>>>
>>> next - stable is yours.
>>>
>>
>> Is it? Ok, for now, until a -stable maintainer is appointed eventually.
>>
>>> If you like review of stable backports, please tag those commits as
>>> [stable] or so. And maybe also explain any deviation.
>>>
>>
>> The way -stable was used is obviously different in the new process, and
>> the patches were consistent with the previous process, so let's drop
>> those patches if you can't review them, I'll rebase over the upcoming
>> -next branch to make it easier for everyone.
>>
>>> My vision would be that only patches that are in next/master would make
>>> it to stable.
>>
>> I understand that you want to align with the basics of the mainline
>> kernel workflow, I'm ok with this. Please set (or re-purpose) the
>> (existing) git branches on gitlab the way you want to, to clarify the
>> situation.
>>
>>   Ideally, no or only minor tweaking should be needed to
>>> such commits for the backport.
>>>
>>
>> This should be ok in most cases. Unfortunately, the XENO_DEBUG() fixes
>> are going to be an exception because they have to spread all over the
>> map - unless they don't go to stable, which could be an issue since the
>> corresponding bug may produce bad kernels.
>>
> 
> It seems we addressed most of the XENO_DEBUG cases in next. Are the

Yes, there is only one missing:

kernel/cobalt/intr.c:#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)

> related commits no stable candidates because they do more? If they are,
> I would start with backporting them, and then address what remains.
> 

The related commits are in essence -stable candidates, done at a time
when -stable was actually the code base the -next branch would be
rebased on periodically (we used to inherit bug fixes in next, not the
other way around via backports).

The main issue with the conflicting patch stems from the fact that the
XENO_DEBUG() changes were originally based on the -next branch, and I
did not backport them to -stable. Now I'm kind of doing so, plugging the
last hole in the process for -stable. That should be done for -next, ...
next.

I'll submit the proper patches.

-- 
Philippe.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
  2018-09-04 13:31 [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals Philippe Gerum
  2018-09-04 17:48 ` Jan Kiszka
@ 2018-09-24 13:52 ` Jan Kiszka
  2018-09-24 14:03   ` Philippe Gerum
  1 sibling, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2018-09-24 13:52 UTC (permalink / raw)
  To: Philippe Gerum, xenomai

On 04.09.18 15:31, Philippe Gerum wrote:
> Testing debug switches using '#if XENO_DEBUG(<feat>)' may confuse the
> build system's dependency tracker, breaking the build, or even
> producing a non-bootable kernel in extreme cases. Typically, turning
> on/off the lock debugging code - usually in absence of any other
> change - may cause the following error to pop up at the link stage:
> 
> ---
> MODPOST vmlinux.o
> kernel/xenomai/lock.o: In function `____xnlock_get':
> linux/include/xenomai/cobalt/kernel/lock.h:184: undefined reference to `xnlock_dbg_prepare_acquire'
> linux/include/xenomai/cobalt/kernel/lock.h:189: undefined reference to `xnlock_dbg_acquired'
> kernel/xenomai/lock.o: In function `____xnlock_put':
> linux/include/xenomai/cobalt/kernel/lock.h:196: undefined reference to `xnlock_dbg_release'
> ---
> 
> Replace all occurrences of the confusing conditionals with the
> equivalent expression explicitly testing the debug symbol.
> 
> Signed-off-by: Philippe Gerum <rpm@xenomai.org>
> ---
>   include/cobalt/kernel/assert.h         |  2 +-
>   include/cobalt/kernel/lock.h           |  6 +++---
>   include/cobalt/kernel/sched-sporadic.h |  2 +-
>   include/cobalt/kernel/synch.h          |  2 +-
>   include/cobalt/kernel/tree.h           |  2 +-
>   kernel/cobalt/bufd.c                   | 10 +++++-----
>   kernel/cobalt/debug.c                  |  2 +-
>   kernel/cobalt/intr.c                   |  2 +-
>   kernel/cobalt/posix/process.c          |  2 +-
>   kernel/cobalt/procfs.c                 |  2 +-
>   kernel/cobalt/sched-sporadic.c         |  4 ++--
>   kernel/cobalt/synch.c                  |  2 +-
>   12 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/include/cobalt/kernel/assert.h b/include/cobalt/kernel/assert.h
> index 2d2d65396..86d0a480f 100644
> --- a/include/cobalt/kernel/assert.h
> +++ b/include/cobalt/kernel/assert.h
> @@ -63,7 +63,7 @@
>   #define realtime_cpu_only()	XENO_BUG_ON(CONTEXT, !xnsched_supported_cpu(ipipe_processor_id()))
>   #define thread_only()		XENO_BUG_ON(CONTEXT, xnsched_interrupt_p())
>   #define irqoff_only()		XENO_BUG_ON(CONTEXT, hard_irqs_disabled() == 0)
> -#if XENO_DEBUG(LOCKING)
> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>   #define atomic_only()		XENO_BUG_ON(CONTEXT, (xnlock_is_owner(&nklock) && hard_irqs_disabled()) == 0)
>   #define preemptible_only()	XENO_BUG_ON(CONTEXT, xnlock_is_owner(&nklock) || hard_irqs_disabled())
>   #else
> diff --git a/include/cobalt/kernel/lock.h b/include/cobalt/kernel/lock.h
> index 36f81688a..99c7848e8 100644
> --- a/include/cobalt/kernel/lock.h
> +++ b/include/cobalt/kernel/lock.h
> @@ -63,7 +63,7 @@ typedef unsigned long spl_t;
>    */
>   #define spltest()   ipipe_test_head()
>   
> -#if XENO_DEBUG(LOCKING)
> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>   
>   struct xnlock {
>   	unsigned owner;
> @@ -152,7 +152,7 @@ static inline int xnlock_dbg_release(struct xnlock *lock)
>   
>   #endif /* !XENO_DEBUG(LOCKING) */
>   
> -#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)
> +#if defined(CONFIG_SMP) || defined(CONFIG_XENO_OPT_DEBUG_LOCKING)
>   
>   #define xnlock_get(lock)		__xnlock_get(lock  XNLOCK_DBG_CONTEXT)
>   #define xnlock_put(lock)		__xnlock_put(lock  XNLOCK_DBG_CONTEXT)
> @@ -209,7 +209,7 @@ int ___xnlock_get(struct xnlock *lock /*, */ XNLOCK_DBG_CONTEXT_ARGS);
>   void ___xnlock_put(struct xnlock *lock /*, */ XNLOCK_DBG_CONTEXT_ARGS);
>   #endif /* out of line xnlock */
>   
> -#if XENO_DEBUG(LOCKING)
> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>   /* Disable UP-over-SMP kernel optimization in debug mode. */
>   #define __locking_active__  1
>   #else
> diff --git a/include/cobalt/kernel/sched-sporadic.h b/include/cobalt/kernel/sched-sporadic.h
> index 0eade47b0..50ca406c4 100644
> --- a/include/cobalt/kernel/sched-sporadic.h
> +++ b/include/cobalt/kernel/sched-sporadic.h
> @@ -56,7 +56,7 @@ struct xnsched_sporadic_data {
>   };
>   
>   struct xnsched_sporadic {
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	unsigned long drop_retries;
>   #endif
>   };
> diff --git a/include/cobalt/kernel/synch.h b/include/cobalt/kernel/synch.h
> index f5ac217ed..08056055a 100644
> --- a/include/cobalt/kernel/synch.h
> +++ b/include/cobalt/kernel/synch.h
> @@ -101,7 +101,7 @@ static inline struct xnthread *xnsynch_owner(struct xnsynch *synch)
>   #define xnsynch_owner_check(synch, thread) \
>   	xnsynch_fast_owner_check((synch)->fastlock, thread->handle)
>   
> -#if XENO_DEBUG(MUTEX_RELAXED)
> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>   
>   void xnsynch_detect_relaxed_owner(struct xnsynch *synch,
>   				  struct xnthread *sleeper);
> diff --git a/include/cobalt/kernel/tree.h b/include/cobalt/kernel/tree.h
> index 9c751cc02..c52ee3220 100644
> --- a/include/cobalt/kernel/tree.h
> +++ b/include/cobalt/kernel/tree.h
> @@ -83,7 +83,7 @@ struct xnid *xnid_fetch(struct rb_root *t, xnkey_t key)
>   
>   static inline int xnid_remove(struct rb_root *t, struct xnid *xnid)
>   {
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	if (xnid_fetch(t, xnid->key) != xnid)
>   		return -ENOENT;
>   #endif
> diff --git a/kernel/cobalt/bufd.c b/kernel/cobalt/bufd.c
> index 9b41b6ad6..3b79505d5 100644
> --- a/kernel/cobalt/bufd.c
> +++ b/kernel/cobalt/bufd.c
> @@ -493,7 +493,7 @@ ssize_t xnbufd_unmap_uread(struct xnbufd *bufd)
>   {
>   	preemptible_only();
>   
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	bufd->b_ptr = (caddr_t)-1;
>   #endif
>   	return bufd->b_off;
> @@ -551,7 +551,7 @@ ssize_t xnbufd_unmap_uwrite(struct xnbufd *bufd)
>   	if (bufd->b_len > sizeof(bufd->b_buf))
>   		xnfree(bufd->b_carry);
>   done:
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	bufd->b_ptr = (caddr_t)-1;
>   #endif
>   	return ret ?: (ssize_t)len;
> @@ -592,7 +592,7 @@ EXPORT_SYMBOL_GPL(xnbufd_unmap_uwrite);
>    */
>   void xnbufd_invalidate(struct xnbufd *bufd)
>   {
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	bufd->b_ptr = (caddr_t)-1;
>   #endif
>   	if (bufd->b_carry) {
> @@ -620,7 +620,7 @@ EXPORT_SYMBOL_GPL(xnbufd_invalidate);
>    */
>   ssize_t xnbufd_unmap_kread(struct xnbufd *bufd)
>   {
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	bufd->b_ptr = (caddr_t)-1;
>   #endif
>   	return bufd->b_off;
> @@ -643,7 +643,7 @@ EXPORT_SYMBOL_GPL(xnbufd_unmap_kread);
>    */
>   ssize_t xnbufd_unmap_kwrite(struct xnbufd *bufd)
>   {
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	bufd->b_ptr = (caddr_t)-1;
>   #endif
>   	return bufd->b_off;
> diff --git a/kernel/cobalt/debug.c b/kernel/cobalt/debug.c
> index 74022c516..af9a95ab6 100644
> --- a/kernel/cobalt/debug.c
> +++ b/kernel/cobalt/debug.c
> @@ -549,7 +549,7 @@ static inline void init_thread_relax_trace(struct xnthread *thread)
>   
>   #endif /* !XENO_OPT_DEBUG_TRACE_RELAX */
>   
> -#if XENO_DEBUG(LOCKING)
> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>   
>   void xnlock_dbg_prepare_acquire(unsigned long long *start)
>   {
> diff --git a/kernel/cobalt/intr.c b/kernel/cobalt/intr.c
> index fb27a3295..69faa23ba 100644
> --- a/kernel/cobalt/intr.c
> +++ b/kernel/cobalt/intr.c
> @@ -531,7 +531,7 @@ static inline void xnintr_irq_detach(struct xnintr *intr)
>   #else /* !CONFIG_XENO_OPT_SHIRQ */
>   
>   struct xnintr_vector {
> -#if defined(CONFIG_SMP) || XENO_DEBUG(LOCKING)
> +#if defined(CONFIG_SMP) || defined(CONFIG_XENO_OPT_DEBUG_LOCKING)
>   	DECLARE_XNLOCK(lock);
>   #endif /* CONFIG_SMP || XENO_DEBUG(LOCKING) */
>   } ____cacheline_aligned_in_smp;
> diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
> index dbcb81ef8..50f752f13 100644
> --- a/kernel/cobalt/posix/process.c
> +++ b/kernel/cobalt/posix/process.c
> @@ -751,7 +751,7 @@ static inline int handle_exception(struct ipipe_trap_data *d)
>   	 * running in primary mode, move it to the Linux domain,
>   	 * leaving the kernel process the exception.
>   	 */
> -#if XENO_DEBUG(COBALT) || XENO_DEBUG(USER)
> +#if defined(CONFIG_XENO_OPT_DEBUG_COBALT) || defined(CONFIG_XENO_OPT_DEBUG_USER)
>   	if (!user_mode(d->regs)) {
>   		xntrace_panic_freeze();
>   		printk(XENO_WARNING
> diff --git a/kernel/cobalt/procfs.c b/kernel/cobalt/procfs.c
> index ade13cf1c..d1f01f038 100644
> --- a/kernel/cobalt/procfs.c
> +++ b/kernel/cobalt/procfs.c
> @@ -27,7 +27,7 @@
>   #include <xenomai/version.h>
>   #include "debug.h"
>   
> -#if XENO_DEBUG(LOCKING)
> +#ifdef CONFIG_XENO_OPT_DEBUG_LOCKING
>   
>   static int lock_vfile_show(struct xnvfile_regular_iterator *it, void *data)
>   {
> diff --git a/kernel/cobalt/sched-sporadic.c b/kernel/cobalt/sched-sporadic.c
> index d2a1f76d0..9cbfa5be3 100644
> --- a/kernel/cobalt/sched-sporadic.c
> +++ b/kernel/cobalt/sched-sporadic.c
> @@ -24,7 +24,7 @@
>   
>   static void sporadic_post_recharge(struct xnthread *thread, xnticks_t budget);
>   
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   
>   static inline void sporadic_note_late_drop(struct xnsched *sched)
>   {
> @@ -236,7 +236,7 @@ static void xnsched_sporadic_init(struct xnsched *sched)
>   	 * share the same priority scale, with the addition of budget
>   	 * management for the sporadic ones.
>   	 */
> -#if XENO_DEBUG(COBALT)
> +#ifdef CONFIG_XENO_OPT_DEBUG_COBALT
>   	sched->pss.drop_retries = 0;
>   #endif
>   }
> diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
> index 7773a08e5..32df635a8 100644
> --- a/kernel/cobalt/synch.c
> +++ b/kernel/cobalt/synch.c
> @@ -902,7 +902,7 @@ void xnsynch_release_all_ownerships(struct xnthread *thread)
>   }
>   EXPORT_SYMBOL_GPL(xnsynch_release_all_ownerships);
>   
> -#if XENO_DEBUG(MUTEX_RELAXED)
> +#ifdef CONFIG_XENO_OPT_DEBUG_MUTEX_RELAXED
>   
>   /*
>    * Detect when a thread is about to sleep on a synchronization
> 

I've picked this one now as stable backport to our 3.0.x branch. I'm not that 
sure we should put the other two ("cobalt/sched: improve watchdog accuracy", 
"cobalt/sched: group high-level init/cleanup code") there as well, though.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
  2018-09-24 13:52 ` Jan Kiszka
@ 2018-09-24 14:03   ` Philippe Gerum
  2018-09-25 11:47     ` Jan Kiszka
  0 siblings, 1 reply; 14+ messages in thread
From: Philippe Gerum @ 2018-09-24 14:03 UTC (permalink / raw)
  To: Jan Kiszka, xenomai

On 09/24/2018 03:52 PM, Jan Kiszka wrote:
> On 04.09.18 15:31, Philippe Gerum wrote:
>> Testing debug switches using '#if XENO_DEBUG(<feat>)' may confuse the
>> build system's dependency tracker, breaking the build, or even
>> producing a non-bootable kernel in extreme cases. Typically, turning
>> on/off the lock debugging code - usually in absence of any other
>> change - may cause the following error to pop up at the link stage:
>>

[snip]

> 
> I've picked this one now as stable backport to our 3.0.x branch. I'm not
> that sure we should put the other two ("cobalt/sched: improve watchdog
> accuracy", "cobalt/sched: group high-level init/cleanup code") there as
> well, though.
> 

We don't need the second one in stable for sure. Merging the first one
there would be beneficial to people who want timeouts shorter than 4s.
E.g. one just cannot use the watchdog feature with a 1s timeout, false
positives would trigger almost immediately even with mildly busy rt systems.

This said, the only report I got (unfortunately privately) about such
issue was only a few weeks ago, so this problem does not look like
affecting many people.

-- 
Philippe.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
  2018-09-24 14:03   ` Philippe Gerum
@ 2018-09-25 11:47     ` Jan Kiszka
  2018-09-25 13:10       ` Philippe Gerum
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2018-09-25 11:47 UTC (permalink / raw)
  To: Philippe Gerum, xenomai

On 24.09.18 16:03, Philippe Gerum wrote:
> On 09/24/2018 03:52 PM, Jan Kiszka wrote:
>> On 04.09.18 15:31, Philippe Gerum wrote:
>>> Testing debug switches using '#if XENO_DEBUG(<feat>)' may confuse the
>>> build system's dependency tracker, breaking the build, or even
>>> producing a non-bootable kernel in extreme cases. Typically, turning
>>> on/off the lock debugging code - usually in absence of any other
>>> change - may cause the following error to pop up at the link stage:
>>>
> 
> [snip]
> 
>>
>> I've picked this one now as stable backport to our 3.0.x branch. I'm not
>> that sure we should put the other two ("cobalt/sched: improve watchdog
>> accuracy", "cobalt/sched: group high-level init/cleanup code") there as
>> well, though.
>>
> 
> We don't need the second one in stable for sure. Merging the first one
> there would be beneficial to people who want timeouts shorter than 4s.
> E.g. one just cannot use the watchdog feature with a 1s timeout, false
> positives would trigger almost immediately even with mildly busy rt systems.
> 
> This said, the only report I got (unfortunately privately) about such
> issue was only a few weeks ago, so this problem does not look like
> affecting many people.
> 

Yeah, and we are getting closer to be able to release 3.1 with that improvement 
included. Therefore, I'm leaning towards declaring it a feature, not a stable fix.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals
  2018-09-25 11:47     ` Jan Kiszka
@ 2018-09-25 13:10       ` Philippe Gerum
  0 siblings, 0 replies; 14+ messages in thread
From: Philippe Gerum @ 2018-09-25 13:10 UTC (permalink / raw)
  To: Jan Kiszka, xenomai

On 09/25/2018 01:47 PM, Jan Kiszka wrote:
> On 24.09.18 16:03, Philippe Gerum wrote:
>> On 09/24/2018 03:52 PM, Jan Kiszka wrote:
>>> On 04.09.18 15:31, Philippe Gerum wrote:
>>>> Testing debug switches using '#if XENO_DEBUG(<feat>)' may confuse the
>>>> build system's dependency tracker, breaking the build, or even
>>>> producing a non-bootable kernel in extreme cases. Typically, turning
>>>> on/off the lock debugging code - usually in absence of any other
>>>> change - may cause the following error to pop up at the link stage:
>>>>
>>
>> [snip]
>>
>>>
>>> I've picked this one now as stable backport to our 3.0.x branch. I'm not
>>> that sure we should put the other two ("cobalt/sched: improve watchdog
>>> accuracy", "cobalt/sched: group high-level init/cleanup code") there as
>>> well, though.
>>>
>>
>> We don't need the second one in stable for sure. Merging the first one
>> there would be beneficial to people who want timeouts shorter than 4s.
>> E.g. one just cannot use the watchdog feature with a 1s timeout, false
>> positives would trigger almost immediately even with mildly busy rt
>> systems.
>>
>> This said, the only report I got (unfortunately privately) about such
>> issue was only a few weeks ago, so this problem does not look like
>> affecting many people.
>>
> 
> Yeah, and we are getting closer to be able to release 3.1 with that
> improvement included. Therefore, I'm leaning towards declaring it a
> feature, not a stable fix.
> 

Ack.


-- 
Philippe.


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-09-25 13:10 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-04 13:31 [Xenomai] [PATCH] cobalt/kernel: always use explicit preprocessor conditionals Philippe Gerum
2018-09-04 17:48 ` Jan Kiszka
2018-09-05  7:44   ` Philippe Gerum
2018-09-05  7:48     ` Philippe Gerum
2018-09-05  7:57       ` Jan Kiszka
2018-09-05  8:16   ` Philippe Gerum
2018-09-05  8:34     ` Jan Kiszka
2018-09-05  9:33       ` Philippe Gerum
2018-09-05  9:44         ` Jan Kiszka
2018-09-05 10:22           ` Philippe Gerum
2018-09-24 13:52 ` Jan Kiszka
2018-09-24 14:03   ` Philippe Gerum
2018-09-25 11:47     ` Jan Kiszka
2018-09-25 13:10       ` Philippe Gerum

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.