On 4 Aug 2020, at 22:47, Junling Ma <junli...@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.
> + 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.
Jess
> simple_unlock(&irqtab.lock);
> splx (s);
> thread_block (NULL);
> diff --git a/device/intr.h b/device/intr.h
> index b1c09e6c..d4f98ca3 100644
> --- a/device/intr.h
> +++ b/device/intr.h
> @@ -44,7 +44,6 @@ struct irqdev {
>
> queue_head_t intr_queue;
> decl_simple_lock_data(, lock);/* a lock to protect the intr_queue */
> - int tot_num_intr; /* Total number of unprocessed interrupts */
>
> /* Machine dependent */
> irq_t irq[NINTR];
> diff --git a/i386/i386/irq.c b/i386/i386/irq.c
> index 4ef1c43f..78375b07 100644
> --- a/i386/i386/irq.c
> +++ b/i386/i386/irq.c
> @@ -68,7 +68,6 @@ void irq_init()
> irqtab.irqdev_ack = irq_eoi;
> queue_init (&irqtab.intr_queue);
> simple_lock_init(&irqtab.lock);
> - irqtab.tot_num_intr = 0;
> for (int i = 0; i < NINTR; ++i)
> irqtab.irq[i] = i;
> }
> --
> 2.28.0.rc1
>
>
