[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] irqdev: remove tot_num_intr
From: |
Junling Ma |
Subject: |
Re: [PATCH] irqdev: remove tot_num_intr |
Date: |
Tue, 4 Aug 2020 15:28:42 -0700 |
Hi Jess,
> On Aug 4, 2020, at 3:22 PM, Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 4 Aug 2020, at 22:47, Junling Ma <junlingm@gmail.com> wrote:
>>
>> The tot_num_intr field is a count of how many deliverable interrupts across
>> all lines. When we move
>> to the scheme of blocking read for request and write for acking, it is
>> possible that an interrupt
>> can happen during a small period that the interrupt is acked, but the read
>> has not happended yet.
>> If it occurs, then the interrupt is not deliverable, so we cannot decrease
>> the interrupts counter
>> and tot_num_intr, otherwise we lose interrupts. But not decreasing
>> tot_num_intr causes an infinit
>> loop in intr_thread. Thus, instead, we remove the tot_num_intr cvounter, and
>> count the number of
>> deliverable interrupts in intr_thread.
>>
>> ---
>> device/intr.c | 10 ++++------
>> device/intr.h | 1 -
>> i386/i386/irq.c | 1 -
>> 3 files changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/device/intr.c b/device/intr.c
>> index b60a6a28..ba86bc2d 100644
>> --- a/device/intr.c
>> +++ b/device/intr.c
>> @@ -98,7 +98,6 @@ deliver_user_intr (int id, void *dev_id, struct pt_regs
>> *regs)
>> __disable_irq (irqtab.irq[id]);
>> e->n_unacked++;
>> e->interrupts++;
>> - irqtab.tot_num_intr++;
>> splx(s);
>>
>> thread_wakeup ((event_t) &intr_thread);
>> @@ -182,15 +181,14 @@ intr_thread (void)
>> printf("irq handler [%d]: removed entry %p\n", id, e);
>> /* if e is the last handler registered for irq ID, then remove
>> the linux irq handler */
>> free_irq(id, e);
>> - if (e->interrupts != 0)
>> - irqtab.tot_num_intr -= e->interrupts;
>> kfree ((vm_offset_t) e, sizeof (*e));
>> e = next;
>> }
>> }
>>
>> /* Now check for interrupts */
>> - while (irqtab.tot_num_intr)
>> + int total = 0;
>
> This is not at the start of a block.
No this is not start of a block. It follows the previous queue_iterate loop. I
just changed the while loop to a do-while loop. It depends on the previous
patches that I sent.
>
>> + do
>> {
>> int del = 0;
>>
>> @@ -210,8 +208,7 @@ intr_thread (void)
>> clear_wait (current_thread (), 0, 0);
>> id = e->id;
>> dst_port = e->dst_port;
>> - e->interrupts--;
>> - irqtab.tot_num_intr--;
>> + total += --e->interrupts;
>>
>> splx (s);
>> deliver_intr (id, dst_port);
>> @@ -237,6 +234,7 @@ intr_thread (void)
>> s = splhigh ();
>> }
>> }
>> + while (total != 0);
>
> This is clearly broken if the loop ever needs to execute more than once.
You are right. I should have initialized the total *inside* the do loop. Will
fix that.
Junling