illumos gate

On systems with fewer CPUs it is likely that the vector for mlxcx_intr_n() and mlxcx_intr_async() can end up on the same CPU.

This can lead to a potential deadlock, as some (a lot of ...) commands which issue commands to HCA will either acquire the CQ or WQ mutex, but the CQ mutex in particular is held by the mlxcx_intr_n(). So, if the CQ mutex acquired by the command coincides with the CQ whose vector routine (mlxcx_intr_n()) is assigned to the same CPU as mlxcx_intr_async(), the mlxcx_intr_n() routine will hold off the execution of mlxcx_intr_async() because there are scheduled at the same PIL.

For example:

THREAD           STATE    SOBJ                COUNT
ffffff001e6d7c20 SLEEP    MUTEX                   1
                 swtch+0x86
                 turnstile_block+0x25b
                 mutex_vector_enter+0x358
                 mlxcx_intr_n+0x128
                 apix_dispatch_by_vector+0x8c
                 apix_dispatch_lowlevel+0x1c

ffffff06ef707460 SLEEP    CV                      1
                 swtch+0x133
                 cv_wait+0x68
                 mlxcx_cmd_wait+0x43
                 mlxcx_cmd_stop_sq+0x111
                 mlxcx_mac_ring_stop+0x20b
                 mac_stop_ring+0x29
                 mac_datapath_teardown+0x5d0
                 mac_client_datapath_teardown+0x56
                 mac_unicast_remove+0x29b
                 aggr_set_port_sdu+0x47
                 aggr_sdu_update+0x91
                 aggr_m_setprop+0x6c
                 mac_set_mtu+0xc4
                 mac_set_prop+0x176
                 drv_ioc_prop_common+0x4dd
                 drv_ioc_setprop+0x1b
                 drv_ioctl+0x1ef
                 cdev_ioctl+0x2b
                 spec_ioctl+0x45
                 fop_ioctl+0x5b
                 ioctl+0x153
                 _sys_sysenter_post_swapgs+0x14f

ffffff0021952c20 SLEEP    MUTEX                   1
                 swtch+0x133
                 turnstile_block+0x25b
                 mutex_vector_enter+0x358
                 mlxcx_mac_ring_tx+0x168
                 mac_hwring_tx+0x1c
                 mac_hwring_send_priv+0x90
                 aggr_ring_tx+0x1a
                 mac_hwring_tx+0x1c
                 mac_tx_send+0x63c
                 mac_tx_soft_ring_process+0x89
                 mac_tx_aggr_mode+0x8c
                 mac_tx+0xdb
                 str_mdata_fastpath_put+0x8e
                 ip_xmit+0x843
                 ire_send_wire_v4+0x345
                 conn_ip_output+0x1d4
                 tcp_send_data+0x58
                 tcp_input_data+0x1f5a
                 squeue_enter+0x3f9
                 ip_fanout_v4+0xbca
                 ip_input_local_v4+0xc6
                 ire_recv_local_v4+0x131
                 ill_input_short_v4+0x3ff
                 ip_input_common_v4+0x23f
                 ip_input+0x1f
                 mac_rx_soft_ring_drain+0x115
                 mac_soft_ring_worker+0xa1
                 thread_start+0xb

and the held off interrupt routine:

> fffffffffbc48000::cpuinfo -v        
 ID ADDR             FLG NRUN BSPL PRI RNRN KRNRN SWITCH THREAD           PROC
  0 fffffffffbc48000  1f    1    6  -1   no    no t-1    ffffff001e605c20 (idle)
                       |    |    |
            RUNNING <--+    |    +--> PIL THREAD
              READY         |           6 ffffff001e6d7c20
           QUIESCED         |
             EXISTS         +-->  PRI THREAD           PROC
             ENABLE                60 ffffff002002ac20 sched

> apixs/J
apixs:
apixs:          ffffff06d919c000 
> ffffff06d919c000::print -t apix_impl_t
apix_impl_t {
    processorid_t x_cpuid = 0
    uint16_t x_intr_pending = 0x40
    struct autovec *[16] x_intr_head = [ 0, 0, 0, 0, 0, 0, 0xffffff06dda435a0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ]
    struct autovec *[16] x_intr_tail = [ 0, 0, 0, 0, 0, 0, 0xffffff06dda435a0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ]
    apix_vector_t *x_obsoletes = 0
    apix_vector_t *[256] x_vectbl = [ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ... ]
    lock_t x_lock = 0
}
> 0xffffff06dda435a0::print -t struct autovec
struct autovec {
    struct autovec *av_link = 0
    uint_t (*)() av_vector = mlxcx_intr_async
    caddr_t av_intarg1 = 0xffffff06e4ab7dc0
    caddr_t av_intarg2 = 0xffffff06e90ea900
    uint64_t *av_ticksp = 0xffffff06e92af538
    uint_t av_prilevel = 0x6
    void *av_intr_id = 0xffffff06dd6179a8
    dev_info_t *av_dip = 0xffffff06dd5d0d50
    ushort_t av_flags = 0x100
    struct autovec *av_ipl_link = 0
}

Thread ffffff001e6d7c20 is block because it can't get the CQ lock, which is held by ffffff06ef707460 which requires the mlxcx_intr_async() to run to complete the command and then it will release the CQ mutex.

A secondary effect is then and tx/rx which relies on vectors on the same CPU will be blocked too.

The CQ and WQ mutex are used to maintain consistency in fields which are only likely to change through user interaction (stopping/starting interface etc). My proposed solution is to set the mlxcx_intr_async() to run at a higher PIL, I am concerned releasing CQ/WQ locks in specific code paths may expose to bugs and we may not cover all the scenarios in which lock manipulation needs to change.