Re: em0+msi related panic

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
> 
> _at__at_ -275,7 +275,7 _at__at_ 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);
> _at__at_ -1307,7 +1307,7 _at__at_ em_handle_rxtx(void *context, int pendin
>   *  Fast Interrupt Service routine
>   *
>   *********************************************************************/
> -static void
> +static int
>  em_intr_fast(void *arg)
>  {
>         struct adapter  *adapter = arg;
> 
> _at__at_ -2173,8 +2174,8 _at__at_ 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.