Re: [Xenomai-core] [RFC][PATCH] shirq locking rework

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1: Type: text/plain, Size: 1790 bytes --]

Dmitry Adamushko wrote:
> Hello Jan,
> 
>> Well, I hate nested locks when it comes to real-time, but in this case I
>> really found no simpler approach. There is the risk of deadlocks via
>>
>> IRQ:            xnintr_shirq::lock -> handler -> nklock vs.
>> Application:    nklock -> xnintr_attach/detach -> xnintr_shirq::lock,
> 
> it's also relevant for the current code - xnintr_attach/detach() must
> not be called while holding the 'nklock'.

That's good, no new restriction (the existing one will be documented now).

> 
> I think, your approach should work as well.. provided we have only a
> single reader vs. a single writter contention case, which seems to be
> the case here ('intrlock' is responsible for synchronization between

Single writer is ensured by intrlock, single reader comes from the
per-IRQ scope of the inner lock.

> xnintr_attach/detach()).. your approach does increase a worst case
> length of the lock(&intrlock) --> unlock(&intrlock) section... but
> that's unlikely to be critical.
> 
> I think, the patch I sent before addresses a reported earlier case
> with xnintr_edge_shirq_handler().. but your approach does make code
> shorter (and likely simpler), right? I don't see any problems it would
> possibly cause (maybe I'm missing smth yet :)

I must confess I didn't get your idea immediately. Later on (cough,
after hacking my own patch, cough) I had a closer look, and it first
appeared fairly nice, despite the additional "ifs". But then I realised
that "end == old_end" may produce false positives in case we have
several times the same (and only the same) IRQ in a row. So, I'm afraid
we have to live with only one candidate. :->

OK, will point our bug reporter to that patch now and ask for testing.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]