Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 19 Jun 2006 15:51:12 +0200
From:      "Attilio Rao" <attilio@freebsd.org>
To:        "Paolo Pisati" <piso@freebsd.org>, perforce@freebsd.org
Subject:   Re: PERFORCE change 99599 for review
Message-ID:  <3bbf2fe10606190651i2135e03fr3572504208a31fa2@mail.gmail.com>
In-Reply-To: <200606191326.k5JDQSKI069433@repoman.freebsd.org>
References:  <200606191326.k5JDQSKI069433@repoman.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
2006/6/19, Paolo Pisati <piso@freebsd.org>:
> http://perforce.freebsd.org/chv.cgi?CH=99599
>
> Change 99599 by piso@piso_newluxor on 2006/06/19 13:26:07
>
>        Turn interrupt filtering execution MD code into MI code:
>        now i've to convert all the remaining archs.
>
> Affected files ...
>
> .. //depot/projects/soc2006/intr_filter/i386/i386/intr_machdep.c#3 edit
> .. //depot/projects/soc2006/intr_filter/kern/kern_intr.c#3 edit
> .. //depot/projects/soc2006/intr_filter/sys/interrupt.h#2 edit
>
> Differences ...
>
> ==== //depot/projects/soc2006/intr_filter/i386/i386/intr_machdep.c#3 (text+ko) ====
>
> @@ -170,9 +170,7 @@
>  {
>        struct thread *td;
>        struct intr_event *ie;
> -       struct intr_handler *ih;
> -       int error, vector, thread, ret;
> -       void *arg;
> +       int error, vector, thread;
>
>        td = curthread;
>
> @@ -214,44 +212,7 @@
>        td->td_intr_nesting_level++;
>        thread = 0;
>        critical_enter();
> -       TAILQ_FOREACH(ih, &ie->ie_handlers, ih_next) {
> -               /*
> -                * Execute fast interrupt handlers directly.
> -                * To support clock handlers, if a handler registers
> -                * with a NULL argument, then we pass it a pointer to
> -                * a trapframe as its argument.
> -                */
> -               arg = ((ih->ih_argument == NULL) ? frame : ih->ih_argument);
> -
> -               CTR4(KTR_INTR, "%s: exec %p(%p) for %s", __func__,
> -                    ih->ih_handler, arg, ih->ih_name);
> -
> -               if (ih->ih_flags & IH_FAST) {
> -                       // XXX - actually should call ih_filter()...
> -                       ret = ((driver_filter_t *)ih->ih_handler)(arg);
> -               } else {
> -                       /* old ithread handler */
> -                       thread = 1;
> -                       continue;
> -               }
> -
> -               /* try with the next handler... */
> -               if (ret == FILTER_STRAY)
> -                       continue;
> -
> -               /* interrupt served, get out of here... */
> -               if (ret == FILTER_HANDLED) {
> -                       thread = 0;
> -                       break;
> -               }
> -
> -               /* mark handler for later execution */
> -               if (ret == FILTER_SCHEDULE_THREAD) {
> -                       ih->ih_need = 1;
> -                       thread = 1;
> -               }
> -       }
> -
> +       thread = intr_filter_loop(ie, frame);
>        /*
>         * If there are any threaded handlers that need to run,
>         * mask the source as well as sending it an EOI.  Otherwise,
>
> ==== //depot/projects/soc2006/intr_filter/kern/kern_intr.c#3 (text+ko) ====
>
> @@ -764,6 +764,64 @@
>        }
>  }
>
> +/*
> + * Main loop for interrupt filter.
> + *
> + * Some architectures (i386, amd64 and arm) require the optional frame
> + * parameter, and use it as the main argument for fast handler execution
> + * when ih_argument == NULL.
> + *
> + * Return: 0 (everything done) or 1 (schedule ithread)
> + */
> +
> +int
> +intr_filter_loop(struct intr_event *ie, struct trapframe *frame) {
> +       struct intr_handler *ih;
> +       void *arg;
> +       int ret, ret2 = 0;
> +
> +       TAILQ_FOREACH(ih, &ie->ie_handlers, ih_next) {
> +               /*
> +                * Execute fast interrupt handlers directly.
> +                * To support clock handlers, if a handler registers
> +                * with a NULL argument, then we pass it a pointer to
> +                * a trapframe as its argument.
> +                */
> +               arg = ((ih->ih_argument == NULL) ? frame : ih->ih_argument);
> +
> +               CTR4(KTR_INTR, "%s: exec %p(%p) for %s", __func__,
> +                    ih->ih_handler, arg, ih->ih_name);
> +
> +               if (ih->ih_flags & IH_FAST) {
> +                       // XXX - actually should call ih_filter()...
> +                       ret = ((driver_filter_t *)ih->ih_handler)(arg);
> +               } else {
> +                       /* old ithread handler */
> +                       ret2 = 1;
> +                       continue;
> +               }
> +
> +               /* try with the next handler... */
> +               if (ret == FILTER_STRAY)
> +                       continue;

Maybe you would like to initialize ret before compare since the case
of (ih->ih_flags & IH_FAST) == 0 is not completely consistent, I
think. And, please, keep in mind that style(9) suggests avoiding
variables initialization in their declarations.

Attilio

-- 
Peace can only be achieved by understanding - A. Einstein



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3bbf2fe10606190651i2135e03fr3572504208a31fa2>