ena: Share ena_global_lock between driver instances

In order to use `ena_global_lock` in sysctl context, it must be kept
outside the driver instance's software context, as sysctls can be called
before attach and after detach, leading to lock use before sx_init and
after sx_destroy otherwise.
Solve this issue by turning `ena_global_lock` into a file scope
variable, shared between all instances of the driver and associated
sysctl context, and in turn initialized/destroyed in dedicated
SYSINIT/SYSUNINIT functions.
As a side effect, this change also fixes existing race in the reset
routine, when simultaneously accessing sysctl exposed properties.

Obtained from: Semihalf
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
This commit is contained in:
Artur Rojek 2021-08-12 10:34:28 +02:00 committed by Marcin Wojtas
parent a831466830
commit 07aff471c0
4 changed files with 99 additions and 58 deletions

View file

@ -181,6 +181,8 @@ static ena_vendor_info_t ena_vendor_info_array[] = {
{ 0, 0, 0 }
};
struct sx ena_global_lock;
/*
* Contains pointers to event handlers, e.g. link state chage.
*/
@ -1127,8 +1129,6 @@ ena_update_buf_ring_size(struct ena_adapter *adapter,
int rc = 0;
bool dev_was_up;
ENA_LOCK_LOCK(adapter);
old_buf_ring_size = adapter->buf_ring_size;
adapter->buf_ring_size = new_buf_ring_size;
@ -1163,8 +1163,6 @@ ena_update_buf_ring_size(struct ena_adapter *adapter,
}
}
ENA_LOCK_UNLOCK(adapter);
return (rc);
}
@ -1176,8 +1174,6 @@ ena_update_queue_size(struct ena_adapter *adapter, uint32_t new_tx_size,
int rc = 0;
bool dev_was_up;
ENA_LOCK_LOCK(adapter);
old_tx_size = adapter->requested_tx_ring_size;
old_rx_size = adapter->requested_rx_ring_size;
adapter->requested_tx_ring_size = new_tx_size;
@ -1218,8 +1214,6 @@ ena_update_queue_size(struct ena_adapter *adapter, uint32_t new_tx_size,
}
}
ENA_LOCK_UNLOCK(adapter);
return (rc);
}
@ -1242,8 +1236,6 @@ ena_update_io_queue_nb(struct ena_adapter *adapter, uint32_t new_num)
int rc = 0;
bool dev_was_up;
ENA_LOCK_LOCK(adapter);
dev_was_up = ENA_FLAG_ISSET(ENA_FLAG_DEV_UP, adapter);
old_num = adapter->num_io_queues;
ena_down(adapter);
@ -1273,8 +1265,6 @@ ena_update_io_queue_nb(struct ena_adapter *adapter, uint32_t new_num)
}
}
ENA_LOCK_UNLOCK(adapter);
return (rc);
}
@ -2029,7 +2019,7 @@ ena_up(struct ena_adapter *adapter)
{
int rc = 0;
ENA_LOCK_ASSERT(adapter);
ENA_LOCK_ASSERT();
if (unlikely(device_is_attached(adapter->pdev) == 0)) {
ena_log(adapter->pdev, ERR, "device is not attached!\n");
@ -2148,13 +2138,13 @@ ena_media_status(if_t ifp, struct ifmediareq *ifmr)
struct ena_adapter *adapter = if_getsoftc(ifp);
ena_log(adapter->pdev, DBG, "Media status update\n");
ENA_LOCK_LOCK(adapter);
ENA_LOCK_LOCK();
ifmr->ifm_status = IFM_AVALID;
ifmr->ifm_active = IFM_ETHER;
if (!ENA_FLAG_ISSET(ENA_FLAG_LINK_UP, adapter)) {
ENA_LOCK_UNLOCK(adapter);
ENA_LOCK_UNLOCK();
ena_log(adapter->pdev, INFO, "Link is down\n");
return;
}
@ -2162,7 +2152,7 @@ ena_media_status(if_t ifp, struct ifmediareq *ifmr)
ifmr->ifm_status |= IFM_ACTIVE;
ifmr->ifm_active |= IFM_UNKNOWN | IFM_FDX;
ENA_LOCK_UNLOCK(adapter);
ENA_LOCK_UNLOCK();
}
static void
@ -2171,9 +2161,9 @@ ena_init(void *arg)
struct ena_adapter *adapter = (struct ena_adapter *)arg;
if (!ENA_FLAG_ISSET(ENA_FLAG_DEV_UP, adapter)) {
ENA_LOCK_LOCK(adapter);
ENA_LOCK_LOCK();
ena_up(adapter);
ENA_LOCK_UNLOCK(adapter);
ENA_LOCK_UNLOCK();
}
}
@ -2195,13 +2185,13 @@ ena_ioctl(if_t ifp, u_long command, caddr_t data)
case SIOCSIFMTU:
if (ifp->if_mtu == ifr->ifr_mtu)
break;
ENA_LOCK_LOCK(adapter);
ENA_LOCK_LOCK();
ena_down(adapter);
ena_change_mtu(ifp, ifr->ifr_mtu);
rc = ena_up(adapter);
ENA_LOCK_UNLOCK(adapter);
ENA_LOCK_UNLOCK();
break;
case SIOCSIFFLAGS:
@ -2213,15 +2203,15 @@ ena_ioctl(if_t ifp, u_long command, caddr_t data)
"ioctl promisc/allmulti\n");
}
} else {
ENA_LOCK_LOCK(adapter);
ENA_LOCK_LOCK();
rc = ena_up(adapter);
ENA_LOCK_UNLOCK(adapter);
ENA_LOCK_UNLOCK();
}
} else {
if ((if_getdrvflags(ifp) & IFF_DRV_RUNNING) != 0) {
ENA_LOCK_LOCK(adapter);
ENA_LOCK_LOCK();
ena_down(adapter);
ENA_LOCK_UNLOCK(adapter);
ENA_LOCK_UNLOCK();
}
}
break;
@ -2246,10 +2236,10 @@ ena_ioctl(if_t ifp, u_long command, caddr_t data)
if ((reinit != 0) &&
((if_getdrvflags(ifp) & IFF_DRV_RUNNING) != 0)) {
ENA_LOCK_LOCK(adapter);
ENA_LOCK_LOCK();
ena_down(adapter);
rc = ena_up(adapter);
ENA_LOCK_UNLOCK(adapter);
ENA_LOCK_UNLOCK();
}
}
@ -2404,7 +2394,7 @@ ena_down(struct ena_adapter *adapter)
{
int rc;
ENA_LOCK_ASSERT(adapter);
ENA_LOCK_ASSERT();
if (!ENA_FLAG_ISSET(ENA_FLAG_DEV_UP, adapter))
return;
@ -3400,12 +3390,12 @@ ena_reset_task(void *arg, int pending)
{
struct ena_adapter *adapter = (struct ena_adapter *)arg;
ENA_LOCK_LOCK(adapter);
ENA_LOCK_LOCK();
if (likely(ENA_FLAG_ISSET(ENA_FLAG_TRIGGER_RESET, adapter))) {
ena_destroy_device(adapter, false);
ena_restore_device(adapter);
}
ENA_LOCK_UNLOCK(adapter);
ENA_LOCK_UNLOCK();
}
/**
@ -3434,8 +3424,6 @@ ena_attach(device_t pdev)
adapter = device_get_softc(pdev);
adapter->pdev = pdev;
ENA_LOCK_INIT(adapter);
/*
* Set up the timer service - driver is responsible for avoiding
* concurrency, as the callout won't be using any locking inside.
@ -3677,19 +3665,19 @@ ena_detach(device_t pdev)
ether_ifdetach(adapter->ifp);
/* Stop timer service */
ENA_LOCK_LOCK(adapter);
ENA_LOCK_LOCK();
callout_drain(&adapter->timer_service);
ENA_LOCK_UNLOCK(adapter);
ENA_LOCK_UNLOCK();
/* 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);
ENA_LOCK_LOCK(adapter);
ENA_LOCK_LOCK();
ena_down(adapter);
ena_destroy_device(adapter, true);
ENA_LOCK_UNLOCK(adapter);
ENA_LOCK_UNLOCK();
/* Restore unregistered sysctl queue nodes. */
ena_sysctl_update_queue_node_nb(adapter, adapter->num_io_queues,
@ -3723,8 +3711,6 @@ ena_detach(device_t pdev)
ena_com_delete_host_info(ena_dev);
ENA_LOCK_DESTROY(adapter);
if_free(adapter->ifp);
free(ena_dev->bus, M_DEVBUF);
@ -3790,6 +3776,20 @@ static void ena_notification(void *adapter_data,
}
}
static void
ena_lock_init(void *arg)
{
ENA_LOCK_INIT();
}
SYSINIT(ena_lock_init, SI_SUB_LOCK, SI_ORDER_FIRST, ena_lock_init, NULL);
static void
ena_lock_uninit(void *arg)
{
ENA_LOCK_DESTROY();
}
SYSUNINIT(ena_lock_uninit, SI_SUB_LOCK, SI_ORDER_FIRST, ena_lock_uninit, NULL);
/**
* This handler will called for unknown event group or unimplemented handlers
**/

View file

@ -401,8 +401,6 @@ struct ena_adapter {
struct resource *msix;
int msix_rid;
struct sx global_lock;
/* MSI-X */
struct msix_entry *msix_entries;
int msix_vecs;
@ -482,17 +480,18 @@ struct ena_adapter {
#define ENA_RING_MTX_ASSERT(_ring) \
mtx_assert(&(_ring)->ring_mtx, MA_OWNED)
#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)
#define ENA_LOCK_ASSERT(adapter) \
sx_assert(&(adapter)->global_lock, SA_XLOCKED)
#define ENA_LOCK_INIT() \
sx_init(&ena_global_lock, "ENA global lock")
#define ENA_LOCK_DESTROY() sx_destroy(&ena_global_lock)
#define ENA_LOCK_LOCK() sx_xlock(&ena_global_lock)
#define ENA_LOCK_UNLOCK() sx_unlock(&ena_global_lock)
#define ENA_LOCK_ASSERT() sx_assert(&ena_global_lock, SA_XLOCKED)
#define clamp_t(type, _x, min, max) min_t(type, max_t(type, _x, min), max)
#define clamp_val(val, lo, hi) clamp_t(__typeof(val), val, lo, hi)
extern struct sx ena_global_lock;
static inline int ena_mbuf_count(struct mbuf *mbuf)
{
int count = 1;

View file

@ -278,7 +278,7 @@ ena_netmap_reg(struct netmap_adapter *na, int onoff)
enum txrx t;
int rc, i;
ENA_LOCK_LOCK(adapter);
ENA_LOCK_LOCK();
ENA_FLAG_CLEAR_ATOMIC(ENA_FLAG_TRIGGER_RESET, adapter);
ena_down(adapter);
@ -315,7 +315,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);
}
ENA_LOCK_UNLOCK(adapter);
ENA_LOCK_UNLOCK();
return (rc);
}

View file

@ -442,6 +442,12 @@ ena_sysctl_buf_ring_size(SYSCTL_HANDLER_ARGS)
uint32_t val;
int error;
ENA_LOCK_LOCK();
if (unlikely(!ENA_FLAG_ISSET(ENA_FLAG_DEVICE_RUNNING, adapter))) {
error = EINVAL;
goto unlock;
}
val = 0;
error = sysctl_wire_old_buffer(req, sizeof(val));
if (error == 0) {
@ -449,13 +455,14 @@ ena_sysctl_buf_ring_size(SYSCTL_HANDLER_ARGS)
error = sysctl_handle_32(oidp, &val, 0, req);
}
if (error != 0 || req->newptr == NULL)
return (error);
goto unlock;
if (!powerof2(val) || val == 0) {
ena_log(adapter->pdev, ERR,
"Requested new Tx buffer ring size (%u) is not a power of 2\n",
val);
return (EINVAL);
error = EINVAL;
goto unlock;
}
if (val != adapter->buf_ring_size) {
@ -470,6 +477,9 @@ ena_sysctl_buf_ring_size(SYSCTL_HANDLER_ARGS)
adapter->buf_ring_size);
}
unlock:
ENA_LOCK_UNLOCK();
return (error);
}
@ -480,6 +490,12 @@ ena_sysctl_rx_queue_size(SYSCTL_HANDLER_ARGS)
uint32_t val;
int error;
ENA_LOCK_LOCK();
if (unlikely(!ENA_FLAG_ISSET(ENA_FLAG_DEVICE_RUNNING, adapter))) {
error = EINVAL;
goto unlock;
}
val = 0;
error = sysctl_wire_old_buffer(req, sizeof(val));
if (error == 0) {
@ -487,13 +503,14 @@ ena_sysctl_rx_queue_size(SYSCTL_HANDLER_ARGS)
error = sysctl_handle_32(oidp, &val, 0, req);
}
if (error != 0 || req->newptr == NULL)
return (error);
goto unlock;
if (val < ENA_MIN_RING_SIZE || val > adapter->max_rx_ring_size) {
ena_log(adapter->pdev, ERR,
"Requested new Rx queue size (%u) is out of range: [%u, %u]\n",
val, ENA_MIN_RING_SIZE, adapter->max_rx_ring_size);
return (EINVAL);
error = EINVAL;
goto unlock;
}
/* Check if the parameter is power of 2 */
@ -501,7 +518,8 @@ ena_sysctl_rx_queue_size(SYSCTL_HANDLER_ARGS)
ena_log(adapter->pdev, ERR,
"Requested new Rx queue size (%u) is not a power of 2\n",
val);
return (EINVAL);
error = EINVAL;
goto unlock;
}
if (val != adapter->requested_rx_ring_size) {
@ -517,6 +535,9 @@ ena_sysctl_rx_queue_size(SYSCTL_HANDLER_ARGS)
adapter->requested_rx_ring_size);
}
unlock:
ENA_LOCK_UNLOCK();
return (error);
}
@ -530,18 +551,25 @@ ena_sysctl_io_queues_nb(SYSCTL_HANDLER_ARGS)
uint32_t old_num_queues, tmp = 0;
int error;
ENA_LOCK_LOCK();
if (unlikely(!ENA_FLAG_ISSET(ENA_FLAG_DEVICE_RUNNING, adapter))) {
error = EINVAL;
goto unlock;
}
error = sysctl_wire_old_buffer(req, sizeof(tmp));
if (error == 0) {
tmp = adapter->num_io_queues;
error = sysctl_handle_int(oidp, &tmp, 0, req);
}
if (error != 0 || req->newptr == NULL)
return (error);
goto unlock;
if (tmp == 0) {
ena_log(adapter->pdev, ERR,
"Requested number of IO queues is zero\n");
return (EINVAL);
error = EINVAL;
goto unlock;
}
/*
@ -555,7 +583,8 @@ ena_sysctl_io_queues_nb(SYSCTL_HANDLER_ARGS)
ena_log(adapter->pdev, ERR,
"Requested number of IO queues is higher than maximum "
"allowed (%u)\n", adapter->msix_vecs - ENA_ADMIN_MSIX_VEC);
return (EINVAL);
error = EINVAL;
goto unlock;
}
if (tmp == adapter->num_io_queues) {
ena_log(adapter->pdev, ERR,
@ -574,6 +603,9 @@ ena_sysctl_io_queues_nb(SYSCTL_HANDLER_ARGS)
ena_sysctl_update_queue_node_nb(adapter, old_num_queues, tmp);
}
unlock:
ENA_LOCK_UNLOCK();
return (error);
}
@ -584,19 +616,26 @@ ena_sysctl_eni_metrics_interval(SYSCTL_HANDLER_ARGS)
uint16_t interval;
int error;
ENA_LOCK_LOCK();
if (unlikely(!ENA_FLAG_ISSET(ENA_FLAG_DEVICE_RUNNING, adapter))) {
error = EINVAL;
goto unlock;
}
error = sysctl_wire_old_buffer(req, sizeof(interval));
if (error == 0) {
interval = adapter->eni_metrics_sample_interval;
error = sysctl_handle_16(oidp, &interval, 0, req);
}
if (error != 0 || req->newptr == NULL)
return (error);
goto unlock;
if (interval > ENI_METRICS_MAX_SAMPLE_INTERVAL) {
ena_log(adapter->pdev, ERR,
"ENI metrics update interval is out of range - maximum allowed value: %d seconds\n",
ENI_METRICS_MAX_SAMPLE_INTERVAL);
return (EINVAL);
error = EINVAL;
goto unlock;
}
if (interval == 0) {
@ -611,5 +650,8 @@ ena_sysctl_eni_metrics_interval(SYSCTL_HANDLER_ARGS)
adapter->eni_metrics_sample_interval = interval;
unlock:
ENA_LOCK_UNLOCK();
return (0);
}