All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: Philippe Gerum <rpm@xenomai.org>
Cc: "Mauerer, Wolfgang" <wolfgang.mauerer@siemens.com>,
	Xenomai <xenomai@xenomai.org>
Subject: Re: [Xenomai] [PATCH] Revert "ipipe: ipipe_request_irq(), ipipe_free_irq() are root-only services"
Date: Thu, 20 Sep 2012 17:45:27 +0200	[thread overview]
Message-ID: <505B3A17.3030008@siemens.com> (raw)
In-Reply-To: <505B3333.20507@xenomai.org>

On 2012-09-20 17:16, Philippe Gerum wrote:
> On 09/20/2012 05:07 PM, Jan Kiszka wrote:
>> On 2012-09-20 16:05, Philippe Gerum wrote:
>>>>>> In this light, let's remove those checks nevertheless.
>>>>>> Enabling/disabling the IRQ are separate calls, and those should be
>>>>>> instrumented as those cause the restriction.
>>>>>>
>>>>>
>>>>> I don't see it this way, because we can't predict what will be the
>>>>> constraints we might have for hooking irqs on all archs we will support.
>>>>> Maybe we will have to run more mainline code in some cases. In any case,
>>>>> we have to fold masking into the de-registration code for proper SMP
>>>>> support - this fix was never finalized precisely because we could not
>>>>> guarantee a root calling context in that case.
>>>>>
>>>>> These checks are there to express the fact that calling from non-root is
>>>>> inherently unsafe. We might find a (ugly) way to tag irq descriptors,
>>>>> for knowing whether this is safe to call from non-root and test this
>>>>> conditionally. But at the end of the day, we would still end up with
>>>>> checking for arch-specific constraints in a generic API, which would be
>>>>> wrong by design.
>>>>>
>>>>> I put these checks when refactoring the pipeline API for the very same
>>>>> reason than you agree to update the RTDM spec regarding
>>>>> rtdm_request_irq: no sane code should have called ipipe_virtualize_irq()
>>>>> from a non-root context. This is just about formalizing this fact.
>>>>
>>>> ...at the wrong point. Plus it is breaking our instrumentation. Again:
>>>> this belongs where we can asses the problem better, i.e. in the higher
>>>> layer and in those functions that do break when called from RT context.
>>>
>>> I don't think so.
>>
>> I do agree that the caller of ipipe_request_irq should not be called
>> over RT context. However, ipipe_request_irq itself _is_ called under an
>> RT spin lock with hardirqs disabled, both under Xenomai 2.6 and upcoming
>> 3. And this triggers CONFIG_IPIPE_DEBUG_CONTEXT, clearly showing that
>> things are still broken here. What do you suggest to fix all this?
>>
> 
> Fix the callers in the upcoming Xenomai releases, and provide an open
> coded implementation of ipipe_request_irq in ipipe_virtualize_irq when
> CONFIG_IPIPE_LEGACY is enabled, which will take care of the broken legacy.

Both callers have the same structure, so we can either fix both or none.
Fortunately, we appear to be fine by simply converting intrlock to a
plain Linux mutex. Will check twice later.

Jan

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


      reply	other threads:[~2012-09-20 15:45 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-20 10:37 [Xenomai] [PATCH] Revert "ipipe: ipipe_request_irq(), ipipe_free_irq() are root-only services" Jan Kiszka
2012-09-20 10:49 ` Philippe Gerum
2012-09-20 10:56   ` Jan Kiszka
2012-09-20 10:57     ` Jan Kiszka
2012-09-20 11:06       ` Philippe Gerum
2012-09-20 11:15       ` Jan Kiszka
2012-09-20 11:27         ` Jan Kiszka
2012-09-20 13:01         ` Gilles Chanteperdrix
2012-09-20 13:15           ` Jan Kiszka
2012-09-20 14:12             ` Gilles Chanteperdrix
2012-09-20 15:00               ` Jan Kiszka
2012-09-20 15:13                 ` Gilles Chanteperdrix
2012-09-20 13:10         ` Philippe Gerum
2012-09-20 13:54           ` Jan Kiszka
2012-09-20 14:05             ` Philippe Gerum
2012-09-20 15:07               ` Jan Kiszka
2012-09-20 15:16                 ` Philippe Gerum
2012-09-20 15:45                   ` Jan Kiszka [this message]

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=505B3A17.3030008@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=rpm@xenomai.org \
    --cc=wolfgang.mauerer@siemens.com \
    --cc=xenomai@xenomai.org \
    /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.