ice: fix 'scheduling while atomic' on aux critical err interrupt
authorAlexander Lobakin <alexandr.lobakin@intel.com>
Wed, 23 Mar 2022 12:43:52 +0000 (13:43 +0100)
committerJakub Kicinski <kuba@kernel.org>
Wed, 23 Mar 2022 17:40:40 +0000 (10:40 -0700)
There's a kernel BUG splat on processing aux critical error
interrupts in ice_misc_intr():

[ 2100.917085] BUG: scheduling while atomic: swapper/15/0/0x00010000
...
[ 2101.060770] Call Trace:
[ 2101.063229]  <IRQ>
[ 2101.065252]  dump_stack+0x41/0x60
[ 2101.068587]  __schedule_bug.cold.100+0x4c/0x58
[ 2101.073060]  __schedule+0x6a4/0x830
[ 2101.076570]  schedule+0x35/0xa0
[ 2101.079727]  schedule_preempt_disabled+0xa/0x10
[ 2101.084284]  __mutex_lock.isra.7+0x310/0x420
[ 2101.088580]  ? ice_misc_intr+0x201/0x2e0 [ice]
[ 2101.093078]  ice_send_event_to_aux+0x25/0x70 [ice]
[ 2101.097921]  ice_misc_intr+0x220/0x2e0 [ice]
[ 2101.102232]  __handle_irq_event_percpu+0x40/0x180
[ 2101.106965]  handle_irq_event_percpu+0x30/0x80
[ 2101.111434]  handle_irq_event+0x36/0x53
[ 2101.115292]  handle_edge_irq+0x82/0x190
[ 2101.119148]  handle_irq+0x1c/0x30
[ 2101.122480]  do_IRQ+0x49/0xd0
[ 2101.125465]  common_interrupt+0xf/0xf
[ 2101.129146]  </IRQ>
...

As Andrew correctly mentioned previously[0], the following call
ladder happens:

ice_misc_intr() <- hardirq
  ice_send_event_to_aux()
    device_lock()
      mutex_lock()
        might_sleep()
          might_resched() <- oops

Add a new PF state bit which indicates that an aux critical error
occurred and serve it in ice_service_task() in process context.
The new ice_pf::oicr_err_reg is read-write in both hardirq and
process contexts, but only 3 bits of non-critical data probably
aren't worth explicit synchronizing (and they're even in the same
byte [31:24]).

[0] https://lore.kernel.org/all/YeSRUVmrdmlUXHDn@lunn.ch

Fixes: 348048e724a0e ("ice: Implement iidc operations")
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
Tested-by: Michal Kubiak <michal.kubiak@intel.com>
Acked-by: Tony Nguyen <anthony.l.nguyen@intel.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
drivers/net/ethernet/intel/ice/ice.h
drivers/net/ethernet/intel/ice/ice_main.c

index bea1d1e..2ca8870 100644 (file)
@@ -290,6 +290,7 @@ enum ice_pf_state {
        ICE_LINK_DEFAULT_OVERRIDE_PENDING,
        ICE_PHY_INIT_COMPLETE,
        ICE_FD_VF_FLUSH_CTX,            /* set at FD Rx IRQ or timeout */
+       ICE_AUX_ERR_PENDING,
        ICE_STATE_NBITS         /* must be last */
 };
 
@@ -559,6 +560,7 @@ struct ice_pf {
        wait_queue_head_t reset_wait_queue;
 
        u32 hw_csum_rx_error;
+       u32 oicr_err_reg;
        u16 oicr_idx;           /* Other interrupt cause MSIX vector index */
        u16 num_avail_sw_msix;  /* remaining MSIX SW vectors left unclaimed */
        u16 max_pf_txqs;        /* Total Tx queues PF wide */
index b7e8744..296f9d5 100644 (file)
@@ -2255,6 +2255,19 @@ static void ice_service_task(struct work_struct *work)
                return;
        }
 
+       if (test_and_clear_bit(ICE_AUX_ERR_PENDING, pf->state)) {
+               struct iidc_event *event;
+
+               event = kzalloc(sizeof(*event), GFP_KERNEL);
+               if (event) {
+                       set_bit(IIDC_EVENT_CRIT_ERR, event->type);
+                       /* report the entire OICR value to AUX driver */
+                       swap(event->reg, pf->oicr_err_reg);
+                       ice_send_event_to_aux(pf, event);
+                       kfree(event);
+               }
+       }
+
        if (test_bit(ICE_FLAG_PLUG_AUX_DEV, pf->flags)) {
                /* Plug aux device per request */
                ice_plug_aux_dev(pf);
@@ -3041,17 +3054,9 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
 
 #define ICE_AUX_CRIT_ERR (PFINT_OICR_PE_CRITERR_M | PFINT_OICR_HMC_ERR_M | PFINT_OICR_PE_PUSH_M)
        if (oicr & ICE_AUX_CRIT_ERR) {
-               struct iidc_event *event;
-
+               pf->oicr_err_reg |= oicr;
+               set_bit(ICE_AUX_ERR_PENDING, pf->state);
                ena_mask &= ~ICE_AUX_CRIT_ERR;
-               event = kzalloc(sizeof(*event), GFP_ATOMIC);
-               if (event) {
-                       set_bit(IIDC_EVENT_CRIT_ERR, event->type);
-                       /* report the entire OICR value to AUX driver */
-                       event->reg = oicr;
-                       ice_send_event_to_aux(pf, event);
-                       kfree(event);
-               }
        }
 
        /* Report any remaining unexpected interrupts */