All of lore.kernel.org
 help / color / mirror / Atom feed
From: Philippe Gerum <rpm@xenomai.org>
To: Jan Kiszka <jan.kiszka@domain.hid>
Cc: Xenomai core <Xenomai-core@domain.hid>
Subject: Re: [Xenomai-core] arm: Unprotected access to irq_desc field?
Date: Fri, 29 Oct 2010 10:27:33 +0200	[thread overview]
Message-ID: <1288340853.1816.146.camel@domain.hid> (raw)
In-Reply-To: <4CCA70F1.3010501@domain.hid>

On Fri, 2010-10-29 at 09:00 +0200, Jan Kiszka wrote:
> Am 28.10.2010 21:34, Philippe Gerum wrote:
> > On Thu, 2010-10-28 at 21:15 +0200, Gilles Chanteperdrix wrote:
> >> Jan Kiszka wrote:
> >>> Gilles,
> >>>
> >>> I happened to come across rthal_mark_irq_disabled/enabled on arm. On
> >>> first glance, it looks like these helpers manipulate irq_desc::status
> >>> non-atomically, i.e. without holding irq_desc::lock. Isn't this fragile?
> >>
> >> I have no idea. How do the other architectures do? As far as I know,
> >> this code has been copied from there.
> > 
> > Other archs do the same, simply because once an irq is managed by the
> > hal, it may not be shared in any way with the regular kernel. So locking
> > is pointless.
> 
> Indeed, I missed that all the other archs have this uninlined in hal.c.
> 
> However, this leaves at least a race between xnintr_disable/enable and
> XN_ISR_PROPAGATE (ie. the related Linux path) behind.

I can't see why XN_ISR_PROPAGATE would be involved here. This service
pends an interrupt in the pipeline log.

>  Not sure if it
> matters practically - but risking silent breakage for this micro
> optimization?

It was not meant as an optimization; we may not grab the linux
descriptor lock in this context because we may enter it in primary mode.

>  Is disabling/enabling really in the highly
> latency-critical anywhere? Otherwise, I would suggest to just plug this
> by adding the intended lock for this field.

The caller is expected to manage locking; AFAICS the only one who does
not is the RTAI skin, which is obsolete and removed in 2.6.x, so no big
deal.

> 
> Jan
> 

-- 
Philippe.




  reply	other threads:[~2010-10-29  8:27 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-28 15:38 [Xenomai-core] arm: Unprotected access to irq_desc field? Jan Kiszka
2010-10-28 19:15 ` Gilles Chanteperdrix
2010-10-28 19:34   ` Philippe Gerum
2010-10-29  7:00     ` Jan Kiszka
2010-10-29  8:27       ` Philippe Gerum [this message]
2010-10-29  9:05         ` Jan Kiszka
2010-10-29 12:00           ` Philippe Gerum
2010-10-29 12:03             ` Philippe Gerum
2010-10-29 12:09             ` Jan Kiszka
2010-10-29 12:46               ` Philippe Gerum
2010-10-29 12:58                 ` Philippe Gerum
2010-10-29 13:54                   ` Jan Kiszka
2010-10-28 19:37 ` Gilles Chanteperdrix

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1288340853.1816.146.camel@domain.hid \
    --to=rpm@xenomai.org \
    --cc=Xenomai-core@domain.hid \
    --cc=jan.kiszka@domain.hid \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.