git: 07aff471c0de - main - ena: Share ena_global_lock between driver instances
Marcin Wojtas
mw at FreeBSD.org
Wed Sep 1 23:07:42 UTC 2021
The branch main has been updated by mw:
URL: https://cgit.FreeBSD.org/src/commit/?id=07aff471c0de2de9a1dc5c7749c46b525bdd0201
commit 07aff471c0de2de9a1dc5c7749c46b525bdd0201
Author: Artur Rojek <ar at semihalf.com>
AuthorDate: 2021-08-12 08:34:28 +0000
Commit: Marcin Wojtas <mw at FreeBSD.org>
CommitDate: 2021-09-01 23:06:37 +0000
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.
---
sys/dev/ena/ena.c | 74 ++++++++++++++++++++++++------------------------
sys/dev/ena/ena.h | 17 ++++++-----
sys/dev/ena/ena_netmap.c | 4 +--
sys/dev/ena/ena_sysctl.c | 62 +++++++++++++++++++++++++++++++++-------
4 files changed, 99 insertions(+), 58 deletions(-)
diff --git a/sys/dev/ena/ena.c b/sys/dev/ena/ena.c
index 7dff59043bb2..c1b770926b0f 100644
--- a/sys/dev/ena/ena.c
+++ b/sys/dev/ena/ena.c
@@ -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
**/
diff --git a/sys/dev/ena/ena.h b/sys/dev/ena/ena.h
index c7e952f5b571..0e85cb39b001 100644
--- a/sys/dev/ena/ena.h
+++ b/sys/dev/ena/ena.h
@@ -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;
diff --git a/sys/dev/ena/ena_netmap.c b/sys/dev/ena/ena_netmap.c
index 74cdea6c34fa..1525b1efd954 100644
--- a/sys/dev/ena/ena_netmap.c
+++ b/sys/dev/ena/ena_netmap.c
@@ -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);
}
diff --git a/sys/dev/ena/ena_sysctl.c b/sys/dev/ena/ena_sysctl.c
index c9a5419811a8..cc8dff4af0c0 100644
--- a/sys/dev/ena/ena_sysctl.c
+++ b/sys/dev/ena/ena_sysctl.c
@@ -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);
}
More information about the dev-commits-src-main
mailing list