[Xenomai-core] Re: [patch, minor] irq proc output in intr.c

2006-08-22 Thread Jan Kiszka
Dmitry Adamushko wrote:
 On 19/08/06, Jan Kiszka [EMAIL PROTECTED] wrote:

 just realised that the output of /proc/xenomai/irq is suboptimal (no
 names) under !CONFIG_XENO_OPT_SHIRQ_LEVEL 
 !CONFIG_XENO_OPT_SHIRQ_EDGE...
 
 
 
 It should be ok but just in case, Jan, pls give it a try first. I was not
 able to test a branch with rthal_irq_cookie() (but as I recall it worked
 when it was implemented) as I don't have any driver at hand to be loaded.

Thanks for the patch. I'm posting some comments below.

 
 TIA,
 
 
 Thanks,
 Jan


 
 
 
 
 diff -urp xenomai-SVN/include/nucleus/intr.h xenomai-a/include/nucleus/intr.h
 --- xenomai-SVN/include/nucleus/intr.h2006-07-20 11:09:01.0 
 +0200
 +++ xenomai-a/include/nucleus/intr.h  2006-08-22 09:32:24.0 +0200
 @@ -71,7 +71,9 @@ int xnintr_mount(void);
  
  void xnintr_clock_handler(void);
  
 +#if defined(CONFIG_PROC_FS)  defined(__KERNEL__)
  int xnintr_irq_proc(unsigned int irq, char *str);
 +#endif /* CONFIG_PROC_FS  __KERNEL__ */
  
  /* Public interface. */
  
 diff -urp xenomai-SVN/ksrc/nucleus/intr.c xenomai-a/ksrc/nucleus/intr.c
 --- xenomai-SVN/ksrc/nucleus/intr.c   2006-07-20 12:35:40.0 +0200
 +++ xenomai-a/ksrc/nucleus/intr.c 2006-08-22 09:34:28.0 +0200
 @@ -691,6 +691,7 @@ int xnintr_mount(void)
   return 0;
  }
  
 +#if defined(CONFIG_PROC_FS)  defined(__KERNEL__)
  int xnintr_irq_proc(unsigned int irq, char *str)
  {
   xnintr_shirq_t *shirq;
 @@ -727,6 +728,7 @@ int xnintr_irq_proc(unsigned int irq, ch
  
   return p - str;
  }
 +#endif /* CONFIG_PROC_FS  __KERNEL__ */
  
  #else /* !CONFIG_XENO_OPT_SHIRQ_LEVEL  !CONFIG_XENO_OPT_SHIRQ_EDGE */
  
 @@ -735,10 +737,31 @@ int xnintr_mount(void)
   return 0;
  }
  
 +#if defined(CONFIG_PROC_FS)  defined(__KERNEL__)
  int xnintr_irq_proc(unsigned int irq, char *str)
  {
 - return 0;
 + xnintr_t *intr;
 + char *p = str;

+   spl_t s;

 +
 + if (rthal_virtual_irq_p(irq)) {
 + p += sprintf(p,  [virtual]);
 + return p - str;
 + } else if (irq == XNARCH_TIMER_IRQ) {
 + p += sprintf(p,  %s, nkclock.name);
 + return p - str;
 + }
 +
 + xnlock_get_irqsave(nklock, s);

What's the idea of this lock? I'm asking as xnintr_attach/detach should
not run under nklock, right? So, how can it protect us then?

 +
 + intr = rthal_irq_cookie(rthal_domain, irq);
 + if (intr  *(intr-name))
 + p += sprintf(p, %s, intr-name);
 +
 + xnlock_put_irqrestore(nklock, s);
 +
 + return p - str;
  }
 +#endif /* CONFIG_PROC_FS  __KERNEL__ */
  
  #endif /* CONFIG_XENO_OPT_SHIRQ_LEVEL || CONFIG_XENO_OPT_SHIRQ_EDGE */
  
 
 
 
 
 --- ChangeLog-SVN 2006-08-22 02:14:43.0 +0200
 +++ ChangeLog 2006-08-22 02:19:21.0 +0200
 @@ -10,6 +10,9 @@
  
   * ksrc/nucleus/pipe.c (xnpipe_recv): Re-evaluate the timeout value
   as we iterate over the message acquisition.
 + 
 + * ksrc/nucleus/intr.c (xnintr_irq_proc): Print the names of registered
 + handlers also in case of non-shared interrupts.
  
  2006-08-20  Gilles Chanteperdrix  [EMAIL PROTECTED]
  

Jan



signature.asc
Description: OpenPGP digital signature
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


[Xenomai-core] Re: [patch, minor] irq proc output in intr.c

2006-08-22 Thread Dmitry Adamushko
On 22/08/06, Jan Kiszka [EMAIL PROTECTED] wrote:
Dmitry Adamushko wrote:. diff -urp xenomai-SVN/include/nucleus/intr.h xenomai-a/include/nucleus/intr.h --- xenomai-SVN/include/nucleus/intr.h2006-07-20 11:09:01.0 +0200 +++ xenomai-a/include/nucleus/intr.h2006-08-22 09:32:
24.0 +0200 @@ -71,7 +71,9 @@ int xnintr_mount(void);void xnintr_clock_handler(void); +#if defined(CONFIG_PROC_FS)  defined(__KERNEL__)int xnintr_irq_proc(unsigned int irq, char *str);
 +#endif /* CONFIG_PROC_FS  __KERNEL__ *//* Public interface. */ diff -urp xenomai-SVN/ksrc/nucleus/intr.c xenomai-a/ksrc/nucleus/intr.c --- xenomai-SVN/ksrc/nucleus/intr.c 2006-07-20 12:35:
40.0 +0200 +++ xenomai-a/ksrc/nucleus/intr.c 2006-08-22 09:34:28.0 +0200 @@ -691,6 +691,7 @@ int xnintr_mount(void) return 0;} +#if defined(CONFIG_PROC_FS)  defined(__KERNEL__)
int xnintr_irq_proc(unsigned int irq, char *str){ xnintr_shirq_t *shirq; @@ -727,6 +728,7 @@ int xnintr_irq_proc(unsigned int irq, ch return p - str;}
 +#endif /* CONFIG_PROC_FS  __KERNEL__ */#else /* !CONFIG_XENO_OPT_SHIRQ_LEVEL  !CONFIG_XENO_OPT_SHIRQ_EDGE */ @@ -735,10 +737,31 @@ int xnintr_mount(void)
 return 0;} +#if defined(CONFIG_PROC_FS)  defined(__KERNEL__)int xnintr_irq_proc(unsigned int irq, char *str){ - return 0; + xnintr_t *intr;
 + char *p = str;+ spl_t s;

My bad. I run diff with an older version, the correct one was in another directory.

 + + if (rthal_virtual_irq_p(irq)) {
+
p += sprintf(p, 
[virtual]); + return p - str; + } else if (irq == XNARCH_TIMER_IRQ) {
+
p += sprintf(p,  %s,
nkclock.name); + return p - str; + } + + xnlock_get_irqsave(nklock, s);What's the idea of this lock? I'm asking as xnintr_attach/detach should
not run under nklock, right? So, how can it protect us then?

They can as rthal_critical_enter/exit() is no longer used in rthal_irq_request().

Now I actually recall why I have not used a cookie field to store a
corresponding xnintr_shirq_t object. It's not safe wrt detach ops. and
there is no way to implement correct synchronization involving only the
nucleus layer.
Ok, it's possible to use something similar to
xnintr_shirq_lonk/unlock() but that would require the per-irq array
also for non-irq-shared case and that's something I wanted to avoid in
the first instance. Hmm.. but maybe it's actually not that bad...

So the problem is that the xnint_t object we have got as a cookie can be deleted at any time while we are using it.

intr = rthal_irq_cookie(rthal_domain, irq);
if (intr  *(intr-name))  p += sprintf(p, %s, intr-name);


Hm.. if I am not wrong, the same problem is true for xnintr_irq_handler().

 Have to think a bit more on this...



Jan-- Best regards,Dmitry Adamushko
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core