This is the mail archive of the ecos-patches@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: FIFO DSRs... the 2nd


Hi Stefan,

"Stefan Sommerfeld" <sommerfeld@mikrom.de> writes:
> Hi Sergei,
>>>
>>> I don't think your implementation of call_pending_DSRs_inner() is
>>> save. You cannot reset the dsr_list and dsr_list_tail entries before
>>> running the dsr routines, because the irqs are activated while the dsr
>>> routines run, so another irq with a dsr could happend. This will
>>> result in a change of the dsr order and also dsr could be missed
>>> because the irq->next pointer is changed. The dsr_list and
>>> dsr_list_tail pointer must be current while the dsrs are running.
>>
>> I believe you are wrong here.
>>
>> Under interrupts disabled I take the entire current list of DSRs out of
>> sight of post_dsr() routine (except it can modify 'dsr_count' of an ISR
>> in the list), so if post_dsr() is called while I handle current DSRs, it
>> will add DSR to a fresh list that will later be handled at top level (in
>> the unlock_inner()).
>
> No, i think you're wrong. You save the pointer to the first dsr, but
> not the list. The irq structures are the elements and the next pointer
> can be modified.
>
> Example:
>
> You have 3 dsr's pending on the call_pending_DSRs_inner() call (let's
> call the dsr a,b and c). You save a pointer to dsr a for you dsr
> calling loop. The dsr routine will of dsr a will be called and before
> it returns another irq happened. This is the irq for dsr b. The
> dsr_list will be set to dsr b and and next pointer of dsr b will be
> set to 0 because the dsr_list_tail pointer was 0.

No. If DSR "b" is already in the list I'm handling, its 'dsr_count'
field is non-zero. Should ISR "b" fire, the post_dsr() will simply
increment the "b->dsr_count". It won't change "b->next_dsr".

Please notice that the above is not different from your
implementation. Every ISR has only one "next_dsr" field, so it can't be
put twice into the list, -- that's why "dsr_count" is there in the first
place, -- to prevent the scenario you've just described. I.e., if what
you describe were indeed the case, neither LIFO nor your algorithm
modelled after LIFO one would work correctly.

> You continue to call the dsr routines and after because dsr b don't
> have the next pointer to dsr c anymore, dsr c will never be
> called. This will not happen with my implementation. Please go back to
> my implementation, because it's save and well tested.

Well, I think mine is also well tested. I tried both and both work for
me. Did you actually try my patch and it failed, or are you just
guessing?

BTW, it would be very interesting, at least for me, if your patch work
and mine doesn't, because my patch stress-tests the code in
call_pending_DSRs_inner() and maybe can shed some light on the "missing
DSR" problem(s) others have encountered.

-- Sergei.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]