d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Use single global lock in the ENA driver

Browse Source 
Currently, the driver had 2 global locks - one was sx lock used for
up/down synchronization and the second one was mutex, which was used
for link configuration and timer service callout.

It is better to have single lock for that. We cannot use mutex, as it
can sleep and cause witness errors in up/down configuration, so sx lock
seems to be the only choice.

Callout cannot use sx lock, but the timer service is MP safe, so we just
need to avoid race between ena_down() and ena_detach(). It can be
avoided by acquiring sx lock.

Simple macros were added that are encapsulating implementation of the
lock and makes the code cleaner.

Submitted by:  Michal Krawczyk <mk@semihalf.com>
Obtained from: Semihalf
Sponsored by:  Amazon, Inc.
This commit is contained in:
Marcin Wojtas 2020-05-26 15:39:41 +00:00
parent 7926bc4492
commit 6959869eae
3 changed files with 38 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
sys/dev/ena/ena.c
View File

@ -1907,13 +1907,13 @@ ena_media_status(if_t ifp, struct ifmediareq *ifmr)
struct ena_adapter *adapter = if_getsoftc(ifp);
ena_trace(ENA_DBG, "enter\n");
mtx_lock(&adapter->global_mtx);
ENA_LOCK_LOCK(adapter);
ifmr->ifm_status = IFM_AVALID;
ifmr->ifm_active = IFM_ETHER;
if (!ENA_FLAG_ISSET(ENA_FLAG_LINK_UP, adapter)) {
mtx_unlock(&adapter->global_mtx);
ENA_LOCK_UNLOCK(adapter);
ena_trace(ENA_INFO, "Link is down\n");
return;
}
@ -1921,7 +1921,7 @@ ena_media_status(if_t ifp, struct ifmediareq *ifmr)
ifmr->ifm_status |= IFM_ACTIVE;
ifmr->ifm_active |= IFM_UNKNOWN | IFM_FDX;
mtx_unlock(&adapter->global_mtx);
ENA_LOCK_UNLOCK(adapter);
}
static void
@ -1930,9 +1930,9 @@ ena_init(void *arg)
struct ena_adapter *adapter = (struct ena_adapter *)arg;
if (!ENA_FLAG_ISSET(ENA_FLAG_DEV_UP, adapter)) {
sx_xlock(&adapter->ioctl_sx);
ENA_LOCK_LOCK(adapter);
ena_up(adapter);
sx_unlock(&adapter->ioctl_sx);
ENA_LOCK_UNLOCK(adapter);
}
}
@ -1954,13 +1954,13 @@ ena_ioctl(if_t ifp, u_long command, caddr_t data)
case SIOCSIFMTU:
if (ifp->if_mtu == ifr->ifr_mtu)
break;
sx_xlock(&adapter->ioctl_sx);
ENA_LOCK_LOCK(adapter);
ena_down(adapter);
ena_change_mtu(ifp, ifr->ifr_mtu);
rc = ena_up(adapter);
sx_unlock(&adapter->ioctl_sx);
ENA_LOCK_UNLOCK(adapter);
break;
case SIOCSIFFLAGS:
@ -1972,15 +1972,15 @@ ena_ioctl(if_t ifp, u_long command, caddr_t data)
"ioctl promisc/allmulti\n");
}
} else {
sx_xlock(&adapter->ioctl_sx);
ENA_LOCK_LOCK(adapter);
rc = ena_up(adapter);
sx_unlock(&adapter->ioctl_sx);
ENA_LOCK_UNLOCK(adapter);
}
} else {
if ((if_getdrvflags(ifp) & IFF_DRV_RUNNING) != 0) {
sx_xlock(&adapter->ioctl_sx);
ENA_LOCK_LOCK(adapter);
ena_down(adapter);
sx_unlock(&adapter->ioctl_sx);
ENA_LOCK_UNLOCK(adapter);
}
}
break;
@ -2005,10 +2005,10 @@ ena_ioctl(if_t ifp, u_long command, caddr_t data)
if ((reinit != 0) &&
((if_getdrvflags(ifp) & IFF_DRV_RUNNING) != 0)) {
sx_xlock(&adapter->ioctl_sx);
ENA_LOCK_LOCK(adapter);
ena_down(adapter);
rc = ena_up(adapter);
sx_unlock(&adapter->ioctl_sx);
ENA_LOCK_UNLOCK(adapter);
}
}
@ -3180,10 +3180,10 @@ ena_reset_task(void *arg, int pending)
return;
}
sx_xlock(&adapter->ioctl_sx);
ENA_LOCK_LOCK(adapter);
ena_destroy_device(adapter, false);
ena_restore_device(adapter);
sx_unlock(&adapter->ioctl_sx);
ENA_LOCK_UNLOCK(adapter);
}
/**
@ -3212,11 +3212,13 @@ ena_attach(device_t pdev)
adapter = device_get_softc(pdev);
adapter->pdev = pdev;
mtx_init(&adapter->global_mtx, "ENA global mtx", NULL, MTX_DEF);
sx_init(&adapter->ioctl_sx, "ENA ioctl sx");
ENA_LOCK_INIT(adapter);
/* Set up the timer service */
callout_init_mtx(&adapter->timer_service, &adapter->global_mtx, 0);
/*
* Set up the timer service - driver is responsible for avoiding
* concurrency, as the callout won't be using any locking inside.
*/
callout_init(&adapter->timer_service, true);
adapter->keep_alive_timeout = DEFAULT_KEEP_ALIVE_TO;
adapter->missing_tx_timeout = DEFAULT_TX_CMP_TO;
adapter->missing_tx_max_queues = DEFAULT_TX_MONITORED_QUEUES;
@ -3435,16 +3437,20 @@ ena_detach(device_t pdev)
ether_ifdetach(adapter->ifp);
/* Free reset task and callout */
/* Stop timer service */
ENA_LOCK_LOCK(adapter);
callout_drain(&adapter->timer_service);
ENA_LOCK_UNLOCK(adapter);
/* Release reset task */
while (taskqueue_cancel(adapter->reset_tq, &adapter->reset_task, NULL))
taskqueue_drain(adapter->reset_tq, &adapter->reset_task);
taskqueue_free(adapter->reset_tq);
sx_xlock(&adapter->ioctl_sx);
ENA_LOCK_LOCK(adapter);
ena_down(adapter);
ena_destroy_device(adapter, true);
sx_unlock(&adapter->ioctl_sx);
ENA_LOCK_UNLOCK(adapter);
#ifdef DEV_NETMAP
netmap_detach(adapter->ifp);
@ -3476,8 +3482,7 @@ ena_detach(device_t pdev)
ena_com_delete_host_info(ena_dev);
mtx_destroy(&adapter->global_mtx);
sx_destroy(&adapter->ioctl_sx);
ENA_LOCK_DESTROY(adapter);
if_free(adapter->ifp);

9
sys/dev/ena/ena.h
View File

@ -396,8 +396,7 @@ struct ena_adapter {
struct resource *memory;
struct resource *registers;
struct mtx global_mtx;
struct sx ioctl_sx;
struct sx global_lock;
/* MSI-X */
struct msix_entry *msix_entries;
@ -468,6 +467,12 @@ struct ena_adapter {
#define ENA_RING_MTX_TRYLOCK(_ring) mtx_trylock(&(_ring)->ring_mtx)
#define ENA_RING_MTX_UNLOCK(_ring) mtx_unlock(&(_ring)->ring_mtx)
#define ENA_LOCK_INIT(adapter) \
sx_init(&(adapter)->global_lock, "ENA global lock")
#define ENA_LOCK_DESTROY(adapter) sx_destroy(&(adapter)->global_lock)
#define ENA_LOCK_LOCK(adapter) sx_xlock(&(adapter)->global_lock)
#define ENA_LOCK_UNLOCK(adapter) sx_unlock(&(adapter)->global_lock)
static inline int ena_mbuf_count(struct mbuf *mbuf)
{
int count = 1;

4
sys/dev/ena/ena_netmap.c
View File

@ -277,7 +277,7 @@ ena_netmap_reg(struct netmap_adapter *na, int onoff)
enum txrx t;
int rc, i;
sx_xlock(&adapter->ioctl_sx);
ENA_LOCK_LOCK(adapter);
ENA_FLAG_CLEAR_ATOMIC(ENA_FLAG_TRIGGER_RESET, adapter);
ena_down(adapter);
@ -314,7 +314,7 @@ ena_netmap_reg(struct netmap_adapter *na, int onoff)
ENA_FLAG_SET_ATOMIC(ENA_FLAG_DEV_UP_BEFORE_RESET, adapter);
rc = ena_restore_device(adapter);
}
sx_unlock(&adapter->ioctl_sx);
ENA_LOCK_UNLOCK(adapter);
return (rc);
}