Skip site navigation (1)Skip section navigation (2)
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>