Jan Kiszka wrote:
> Dmitry Adamushko wrote:
>> On 02/07/07, Jan Kiszka <[EMAIL PROTECTED]> wrote:
>>> Dmitry Adamushko wrote:
>>>> On 01/07/07, Jan Kiszka <[EMAIL PROTECTED]> wrote:
>>>>> Hi all,
>>>>>
>>>>> I've just uploaded my rebased patch stack, including specifically the
>>>>> last bits that should go in before -rc1,
>> a few comments:
>>
>> (1) in xnintr_irq_handler()
>>
>> + xnlock_get(&xnirqs[irq].lock);
>> +
>> +#ifdef CONFIG_SMP
>> + /* In SMP case, we have to reload the cookie under the per-IRQ lock
>> + to avoid racing with xnintr_detach. */
>> + intr = rthal_irq_cookie(&rthal_domain, irq);
>> + if (unlikely(!intr)) {
>> + s = 0;
>> + goto unlock_and_exit;
>> + }
>> +#else
>> + intr = cookie;
>> +#endif
>>
>> I think, it would be better to check 'intr' for NULL at this point so
>> to cover 'cookie == NULL' (who knows.. and it's better not to rely on
>> the lower layer here), I guess.
>
> The lower layer just passes what we hand over - and when we do so. Keep
> in mind: this is the non-SMP case, thus we can rely on the atomicity of
> this handler like the one of registering a handler + its cookie (like we
> always did -- unfortunately also for SMP...). For me, checking for NULL
> on UP is redundant code in a hotpath. Should I spread some comment
> regarding this assumption?
>
>> (2) use cases of xnintr_sync_stat_references() in xnintr_irq_detach()
>>
>> +
>> + xnlock_get(&xnirqs[irq].lock);
>> +
>> + err = xnarch_release_irq(intr->irq);
>> + xnintr_sync_stat_references(intr);
>> + xnlock_put(&xnirqs[irq].lock);
>>
>> Why do you call xnintr_sync_stat_references() inside the locked section
>> here?
>> Does xnintr_sync_stat_references() really need to be protected by
>> 'intr->lock' (it's been already detached) ?
>
> That's now my own overcautiousness :). I think we can move it out.
> Someone must sync on some xnintr object while someone else tries to
> re-register an object with very same address -- classical user error,
> nothing we need to catch here.Modified version just went online, see http://www.rts.uni-hannover.de/rtaddon/patches/xenomai/fix-intr-locking-part-ii-v3.patch whitespace-damaged interdiff: --- xenomai/ksrc/nucleus/intr.c +++ xenomai/ksrc/nucleus/intr.c @@ -124,6 +124,7 @@ goto unlock_and_exit; } #else + /* cookie always valid, attach/detach happens with IRQs disabled */ intr = cookie; #endif s = intr->isr(intr); @@ -438,9 +439,10 @@ /* Remove the given interrupt object from the list. */ xnlock_get(&shirq->lock); *p = e->next; - xnintr_sync_stat_references(intr); xnlock_put(&shirq->lock); + xnintr_sync_stat_references(intr); + /* Release the IRQ line if this was the last user */ if (shirq->handlers == NULL) err = xnarch_release_irq(intr->irq); @@ -468,12 +470,11 @@ int err; xnlock_get(&xnirqs[irq].lock); - err = xnarch_release_irq(intr->irq); - xnintr_sync_stat_references(intr); - xnlock_put(&xnirqs[irq].lock); + xnintr_sync_stat_references(intr); + return err; } Jan
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
