nucleus: Convert intrlock to a sleeping Linux lock

PATCH Superseded Jan Kiszka 2 participants 6 messages Oct 17, 2012 – Oct 18, 2012

Share & tools

Jan Kiszka

author

2012-10-17 11:19 UTC #0 lore raw

PATCH 1/1 nucleus: Convert intrlock to a sleeping Linux lock

All users of this lock are supposed to run over Linux context already. Moreover, latest I-pipe patches complain that ipipe_virtualize_irq is called with the Xenomai domain stalled. So convert this lock to a sleeping Linux variant to avoid that alarm.

Signed-off-by Jan Kiszka <[email protected]>

---
 ksrc/nucleus/intr.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

ksrc/nucleus/intr.c +9 ~~-13~~

index c75fcac..4747de1 100644
@@ -39,7 +39,7 @@
 
 #define XNINTR_MAX_UNHANDLED	1000
 
-DEFINE_PRIVATE_XNLOCK(intrlock);
+static DEFINE_BINARY_SEMAPHORE(intrlock);
 
 #ifdef CONFIG_XENO_OPT_STATS
 xnintr_t nkclock;	     /* Only for statistics */

@@ -709,7 +709,6 @@ EXPORT_SYMBOL_GPL(xnintr_destroy);
 int xnintr_attach(xnintr_t *intr, void *cookie)
 {
 	int ret;
-	spl_t s;
 
 	trace_mark(xn_nucleus, irq_attach, "irq %u name %s",
 		   intr->irq, intr->name);

@@ -721,7 +720,7 @@ int xnintr_attach(xnintr_t *intr, void *cookie)
 	xnarch_set_irq_affinity(intr->irq, nkaffinity);
 #endif /* CONFIG_SMP */
 
-	xnlock_get_irqsave(&intrlock, s);
+	down(&intrlock);
 
 	if (__testbits(intr->flags, XN_ISR_ATTACHED)) {
 		ret = -EBUSY;

@@ -735,7 +734,7 @@ int xnintr_attach(xnintr_t *intr, void *cookie)
 	__setbits(intr->flags, XN_ISR_ATTACHED);
 	xnintr_stat_counter_inc();
 out:
-	xnlock_put_irqrestore(&intrlock, s);
+	up(&intrlock);
 
 	return ret;
 }

@@ -775,11 +774,10 @@ EXPORT_SYMBOL_GPL(xnintr_attach);
 int xnintr_detach(xnintr_t *intr)
 {
 	int ret;
-	spl_t s;
 
 	trace_mark(xn_nucleus, irq_detach, "irq %u", intr->irq);
 
-	xnlock_get_irqsave(&intrlock, s);
+	down(&intrlock);
 
 	if (!__testbits(intr->flags, XN_ISR_ATTACHED)) {
 		ret = -EINVAL;

@@ -794,7 +792,7 @@ int xnintr_detach(xnintr_t *intr)
 
 	xnintr_stat_counter_dec();
  out:
-	xnlock_put_irqrestore(&intrlock, s);
+	up(&intrlock);
 
 	return ret;
 }

@@ -920,13 +918,12 @@ int xnintr_query_next(int irq, xnintr_iterator_t *iterator, char *name_buf)
 	xnticks_t last_switch;
 	int cpu_no = iterator->cpu + 1;
 	int err = 0;
-	spl_t s;
 
 	if (cpu_no == xnarch_num_online_cpus())
 		cpu_no = 0;
 	iterator->cpu = cpu_no;
 
-	xnlock_get_irqsave(&intrlock, s);
+	down(&intrlock);
 
 	if (iterator->list_rev != xnintr_list_rev) {
 		err = -EAGAIN;

@@ -969,7 +966,7 @@ int xnintr_query_next(int irq, xnintr_iterator_t *iterator, char *name_buf)
 		iterator->prev = intr;
 
      unlock_and_exit:
-	xnlock_put_irqrestore(&intrlock, s);
+	up(&intrlock);
 
 	return err;
 }

@@ -983,7 +980,6 @@ static inline int format_irq_proc(unsigned int irq,
 				  struct xnvfile_regular_iterator *it)
 {
 	struct xnintr *intr;
-	spl_t s;
 
 	if (irq == XNARCH_TIMER_IRQ) {
 		xnvfile_puts(it, "         [timer]");

@@ -1005,7 +1001,7 @@ static inline int format_irq_proc(unsigned int irq,
 		return 0;
 	}
 
-	xnlock_get_irqsave(&intrlock, s);
+	down(&intrlock);
 
 	intr = xnintr_shirq_first(irq);
 	if (intr) {

@@ -1018,7 +1014,7 @@ static inline int format_irq_proc(unsigned int irq,
 		} while (intr);
 	}
 
-	xnlock_put_irqrestore(&intrlock, s);
+	up(&intrlock);
 
 	return 0;
 }

Signature · 1 line

1.7.3.4

Gilles Chanteperdrix

2012-10-18 04:37 UTC #1 lore raw

Quoted 4 lines · Jan Kiszka

All users of this lock are supposed to run over Linux context already. Moreover, latest I-pipe patches complain that ipipe_virtualize_irq is called with the Xenomai domain stalled. So convert this lock to a sleeping Linux variant to avoid that alarm.

Do we really really want such a change in the stable branch?

Signature · 1 line

                                                                Gilles.

Jan Kiszka

author

2012-10-18 06:11 UTC #2 lore raw

Quoted 9 lines · Gilles Chanteperdrix

Quoted 4 lines · Jan Kiszka
All users of this lock are supposed to run over Linux context already. Moreover, latest I-pipe patches complain that ipipe_virtualize_irq is called with the Xenomai domain stalled. So convert this lock to a sleeping Linux variant to avoid that alarm.
Do we really really want such a change in the stable branch?

Yes because it fixes a bug (that prevents detecting other bugs).

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Gilles Chanteperdrix

2012-10-18 06:21 UTC #3 lore raw

Quoted 9 lines · Jan Kiszka

Quoted 8 lines · Gilles Chanteperdrix
Quoted 4 lines · Jan Kiszka
All users of this lock are supposed to run over Linux context already. Moreover, latest I-pipe patches complain that ipipe_virtualize_irq is called with the Xenomai domain stalled. So convert this lock to a sleeping Linux variant to avoid that alarm.

Quoted 3 lines

Quoted 1 line
Do we really really want such a change in the stable branch?
Yes because it fixes a bug (that prevents detecting other bugs).

The commit log (quoted above) does not say that. It says that it fixes a warning which really looks like a false positive, and that could be disabled when compiling legacy versions of Xenomai.

Signature · 1 line

					    Gilles.

Jan Kiszka

author

2012-10-18 06:30 UTC #4 lore raw

Quoted 10 lines · Gilles Chanteperdrix

Quoted 9 lines · Jan Kiszka
Quoted 8 lines · Gilles Chanteperdrix
Quoted 4 lines · Jan Kiszka
All users of this lock are supposed to run over Linux context already. Moreover, latest I-pipe patches complain that ipipe_virtualize_irq is called with the Xenomai domain stalled. So convert this lock to a sleeping Linux variant to avoid that alarm.

Quoted 5 lines

Quoted 3 lines
Quoted 1 line
Do we really really want such a change in the stable branch?
Yes because it fixes a bug (that prevents detecting other bugs).
The commit log (quoted above) does not say that.

I can improve that.

Quoted 3 lines

It says that it fixes a
warning which really looks like a false positive, and that could be disabled when compiling legacy versions of Xenomai.

Nope as the warning is system wide. And we depend on that self-check to stabilize I-pipe and Xenomai over new kernel versions.

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Jan Kiszka

author

2012-10-18 17:40 UTC #5 lore raw

PATCH v2 1/1 nucleus: Convert intrlock to a sleeping Linux lock

In reply to Jan Kiszka

All users of this lock are supposed to run over Linux context already. Moreover, latest I-pipe patches complain that ipipe_virtualize_irq is called with the Xenomai domain stalled which invalidates that useful internal bug check. So convert this lock to a sleeping Linux variant to avoid the alarm.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- Improved commit message, no code changes

 ksrc/nucleus/intr.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

ksrc/nucleus/intr.c +9 ~~-13~~

index c75fcac..4747de1 100644
@@ -39,7 +39,7 @@
 
 #define XNINTR_MAX_UNHANDLED	1000
 
-DEFINE_PRIVATE_XNLOCK(intrlock);
+static DEFINE_BINARY_SEMAPHORE(intrlock);
 
 #ifdef CONFIG_XENO_OPT_STATS
 xnintr_t nkclock;	     /* Only for statistics */

@@ -709,7 +709,6 @@ EXPORT_SYMBOL_GPL(xnintr_destroy);
 int xnintr_attach(xnintr_t *intr, void *cookie)
 {
 	int ret;
-	spl_t s;
 
 	trace_mark(xn_nucleus, irq_attach, "irq %u name %s",
 		   intr->irq, intr->name);

@@ -721,7 +720,7 @@ int xnintr_attach(xnintr_t *intr, void *cookie)
 	xnarch_set_irq_affinity(intr->irq, nkaffinity);
 #endif /* CONFIG_SMP */
 
-	xnlock_get_irqsave(&intrlock, s);
+	down(&intrlock);
 
 	if (__testbits(intr->flags, XN_ISR_ATTACHED)) {
 		ret = -EBUSY;

@@ -735,7 +734,7 @@ int xnintr_attach(xnintr_t *intr, void *cookie)
 	__setbits(intr->flags, XN_ISR_ATTACHED);
 	xnintr_stat_counter_inc();
 out:
-	xnlock_put_irqrestore(&intrlock, s);
+	up(&intrlock);
 
 	return ret;
 }

@@ -775,11 +774,10 @@ EXPORT_SYMBOL_GPL(xnintr_attach);
 int xnintr_detach(xnintr_t *intr)
 {
 	int ret;
-	spl_t s;
 
 	trace_mark(xn_nucleus, irq_detach, "irq %u", intr->irq);
 
-	xnlock_get_irqsave(&intrlock, s);
+	down(&intrlock);
 
 	if (!__testbits(intr->flags, XN_ISR_ATTACHED)) {
 		ret = -EINVAL;

@@ -794,7 +792,7 @@ int xnintr_detach(xnintr_t *intr)
 
 	xnintr_stat_counter_dec();
  out:
-	xnlock_put_irqrestore(&intrlock, s);
+	up(&intrlock);
 
 	return ret;
 }

@@ -920,13 +918,12 @@ int xnintr_query_next(int irq, xnintr_iterator_t *iterator, char *name_buf)
 	xnticks_t last_switch;
 	int cpu_no = iterator->cpu + 1;
 	int err = 0;
-	spl_t s;
 
 	if (cpu_no == xnarch_num_online_cpus())
 		cpu_no = 0;
 	iterator->cpu = cpu_no;
 
-	xnlock_get_irqsave(&intrlock, s);
+	down(&intrlock);
 
 	if (iterator->list_rev != xnintr_list_rev) {
 		err = -EAGAIN;

@@ -969,7 +966,7 @@ int xnintr_query_next(int irq, xnintr_iterator_t *iterator, char *name_buf)
 		iterator->prev = intr;
 
      unlock_and_exit:
-	xnlock_put_irqrestore(&intrlock, s);
+	up(&intrlock);
 
 	return err;
 }

@@ -983,7 +980,6 @@ static inline int format_irq_proc(unsigned int irq,
 				  struct xnvfile_regular_iterator *it)
 {
 	struct xnintr *intr;
-	spl_t s;
 
 	if (irq == XNARCH_TIMER_IRQ) {
 		xnvfile_puts(it, "         [timer]");

@@ -1005,7 +1001,7 @@ static inline int format_irq_proc(unsigned int irq,
 		return 0;
 	}
 
-	xnlock_get_irqsave(&intrlock, s);
+	down(&intrlock);
 
 	intr = xnintr_shirq_first(irq);
 	if (intr) {

@@ -1018,7 +1014,7 @@ static inline int format_irq_proc(unsigned int irq,
 		} while (intr);
 	}
 
-	xnlock_put_irqrestore(&intrlock, s);
+	up(&intrlock);
 
 	return 0;
 }

Signature · 4 lines

1.7.3.4
-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux