Date: Tue, 27 Feb 2007 15:52:59 +0100 From: Paolo Pisati <piso@FreeBSD.org> To: Goran Gajic <ggajic@afrodita.rcub.bg.ac.yu> Cc: freebsd-current@mx1.freebsd.org Subject: Re: em0+msi related panic Message-ID: <20070227145259.GB1655@tin.it> In-Reply-To: <Pine.LNX.4.64.0702271425110.20218@afrodita.rcub.bg.ac.yu> References: <Pine.LNX.4.64.0702271128420.2901@afrodita.rcub.bg.ac.yu> <Pine.LNX.4.64.0702271425110.20218@afrodita.rcub.bg.ac.yu>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Feb 27, 2007 at 02:32:26PM +0100, Goran Gajic wrote:
>
>
> Hi,
>
> Looking through source and diff of sys/dev/em/if_em.c I have noticed that
>
> @@ -275,7 +275,7 @@ static void em_add_int_delay_sysctl(stru
> static poll_handler_t em_poll;
> static void em_intr(void *);
> #else
> -static void em_intr_fast(void *);
> +static int em_intr_fast(void *);
> static void em_add_int_process_limit(struct adapter *, const char *,
> const char *, int *, int);
> static void em_handle_rxtx(void *context, int pending);
> @@ -1307,7 +1307,7 @@ em_handle_rxtx(void *context, int pendin
> * Fast Interrupt Service routine
> *
> *********************************************************************/
> -static void
> +static int
> em_intr_fast(void *arg)
> {
> struct adapter *adapter = arg;
>
> @@ -2173,8 +2174,8 @@ em_allocate_intr(struct adapter *adapter
>
> #ifdef DEVICE_POLLING
> if (adapter->int_handler_tag == NULL && (error =
> bus_setup_intr(dev,
> - adapter->res_interrupt, INTR_TYPE_NET | INTR_MPSAFE, em_intr,
> adapter,
> - &adapter->int_handler_tag)) != 0) {
> + adapter->res_interrupt, INTR_TYPE_NET | INTR_MPSAFE, NULL,
> em_intr,
> + adapter, &adapter->int_handler_tag)) != 0) {
> device_printf(dev, "Failed to register interrupt
> handler");
> return (error);
> }
>
>
> between revision 1.168 and 1.169 that causes panic. I have switched
> em_intr_fast to be static void and order in bus_setup_intr is incorrect.
> It should be:
>
> if ((error = bus_setup_intr(dev, adapter->res_interrupt,
> INTR_TYPE_NET,NULL, em_intr_fast, adapter,
> &adapter->int_handler_tag)) != 0) {
>
> not
>
> if ((error = bus_setup_intr(dev, adapter->res_interrupt,
> INTR_TYPE_NET, em_intr_fast,NULL adapter,
> &adapter->int_handler_tag)) != 0) {
No, wait, em_intr_fast() was supposed to run as an INTR_FAST (that's
where it gets the suffix '_fast'), and since the bus_setup_intr()
modification to define an INTR_FAST/filter, you use the driver_filter_t
arg, so:
bus_setup_intr(dev, adapter->res_interrupt, INTR_TYPE_NET,
em_intr_fast,NULL adapter, &adapter->int_handler_tag)
is the correct one.
Moreover, all the filter handlers (ex INTR_FAST) return a status
about interrupt handling, so em_intr_fast() prototype and return code
are corrects.
bye,
P.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070227145259.GB1655>