From nobody Tue Jul 26 19:33:26 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4LsnBt6Czgz4WxYr; Tue, 26 Jul 2022 19:33:26 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4LsnBt3Zjhz47hD; Tue, 26 Jul 2022 19:33:26 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1658864006; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=R7ZM4Bs7AFJHTBCVUHllRss2/JsSPSKrHf+MegiZOxU=; b=ppX4lVv9joyLj8K5tS3w/N60V+4I40LKvK4fZMwN9dCa0WzwDNzKklaDdAw1WMri+JaWGT dVVwBktH0m0l6BOywRpWZlOy3LCqLoLEuPwFmulygh53nPF/yeCCRqrNL+UJvLZ9EPIn2Z 0Ybk+huQBJE4J3E484k1IUuDy30VRXBFGhDZ/BUmynshOAD/WbZE5PzGV2nGsKlWe8T4LE MM0jXZs7hoqeoGgElf9eJasdh1RQn/dWz/2zruXP28xb+XwXI02Oe0nP6zcKlhP/NaZXkV nVqP/hwaXU0TOsOV2B4SsEwU+kZvIYVhuN+NZ8jcb3X0C+qsyHa+bcvN5tTgFA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4LsnBt2YcCzpKd; Tue, 26 Jul 2022 19:33:26 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 26QJXQMN033851; Tue, 26 Jul 2022 19:33:26 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 26QJXQKm033850; Tue, 26 Jul 2022 19:33:26 GMT (envelope-from git) Date: Tue, 26 Jul 2022 19:33:26 GMT Message-Id: <202207261933.26QJXQKm033850@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Marcin Wojtas Subject: git: ed037ffb2c05 - stable/12 - ena: Move ena_copy_eni_metrics into separate task List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mw X-Git-Repository: src X-Git-Refname: refs/heads/stable/12 X-Git-Reftype: branch X-Git-Commit: ed037ffb2c05f08ff1c1633ce7f9e92ffe4e6b07 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1658864006; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=R7ZM4Bs7AFJHTBCVUHllRss2/JsSPSKrHf+MegiZOxU=; b=bhp3+lHhSeXSNUUmLfHk2rbR7CDIJA+aHcE413MEZnfHNBwb3AIj3Bwwl40QbeIK55ORqt kn88zZGy21gKxACppSfpx6mCt3m6ox4sT3+wQueGFo6wMsCAjVxmjZit7zNLJ29V+74Wrg rTWS5r9aprlQYkWF4RhRM3f8LCo8Gh0QklpE1kvWdTwJfyEaIgj2QJwTTxKS+7ej2R2G93 TNdrcv11hQJ1zoGOYcJX+UJ2M4FE1wBECFVxbY+xdQsN7pPBIUEXFXzh1r796/hu1zc6R3 dLpXJw9qF9IA5ZsWXv9jyk3knWhD98OFFF3bPpQFjpiM5IL7nBpq2k+EOBRhUw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1658864006; a=rsa-sha256; cv=none; b=F48CuxBVxEpgStI9AURAc+jcCXqcR92QJHEsnv9fwYSAudA/9nIfxePjgSuWpN8P6k1wI8 BiB5ghPvDulS5lXk7T2Fj9QP7KBptwwykBKs+V4c8W+ulr70jNTtYCYpZGgqdCrwFelvEi K2qi/MYxXNpH3mDFotxfLYkc0Hpvu0a7ylMP7UdRGqJ3jiQEDqhv3X4jKXyTl4K4NreUtc gBY4q64oDb7LE9LVZ2nUYbDzHSyQhPHm5bdLzyOx1H4Rki4KZKRyu8PoK4tBkXzbhUhk3x Qg7yFi8BRIZb3SS7zUV6uMmYT/In0gftXtkl7acroluIK2gKLHmZogfbRkCWYQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/12 has been updated by mw: URL: https://cgit.FreeBSD.org/src/commit/?id=ed037ffb2c05f08ff1c1633ce7f9e92ffe4e6b07 commit ed037ffb2c05f08ff1c1633ce7f9e92ffe4e6b07 Author: Dawid Gorecki AuthorDate: 2022-06-10 09:18:10 +0000 Commit: Marcin Wojtas CommitDate: 2022-07-26 19:33:03 +0000 ena: Move ena_copy_eni_metrics into separate task Copying ENI metrics was done in callout context, this caused the driver to panic when sample_interval was set to a value other than 0, as the admin queue call which was executed could sleep while waiting on a condition variable. Taskqueue, unlike callout, allows for sleeping, so moving the function to a separate taskqueue fixes the problem. ena_timer_service is still responsible for scheduling the taskqueue. Stop draining the callout during ena_up/ena_down. This was done to prevent a race between ena_up/down and ena_copy_eni_metrics admin queue calls. Since ena_metrics_task is protected by ENA_LOCK there is no possibility of a race between ena_up/down and ena_metrics_task. Remove a comment about locking in ena_timer_service. With ENI metrics in a separate task this comment became obsolete. Obtained from: Semihalf MFC after: 2 weeks Sponsored by: Amazon, Inc. (cherry picked from commit b899a02ad7330cae3c9bb08ad7975601dc3b9551) --- sys/dev/ena/ena.c | 57 ++++++++++++++++++++++--------------------------------- sys/dev/ena/ena.h | 2 ++ 2 files changed, 25 insertions(+), 34 deletions(-) diff --git a/sys/dev/ena/ena.c b/sys/dev/ena/ena.c index 82f8d42ea0b0..b28156ef2ab2 100644 --- a/sys/dev/ena/ena.c +++ b/sys/dev/ena/ena.c @@ -2104,13 +2104,6 @@ ena_up(struct ena_adapter *adapter) ena_log(adapter->pdev, INFO, "device is going UP\n"); - /* - * ena_timer_service can use functions, which write to the admin queue. - * Those calls are not protected by ENA_LOCK, and because of that, the - * timer should be stopped when bringing the device up or down. - */ - ENA_TIMER_DRAIN(adapter); - /* setup interrupts for IO queues */ rc = ena_setup_io_intr(adapter); if (unlikely(rc != 0)) { @@ -2157,8 +2150,6 @@ ena_up(struct ena_adapter *adapter) ena_unmask_all_io_irqs(adapter); - ENA_TIMER_RESET(adapter); - return (0); err_up_complete: @@ -2168,8 +2159,6 @@ err_up_complete: err_create_queues_with_backoff: ena_free_io_irq(adapter); error: - ENA_TIMER_RESET(adapter); - return (rc); } @@ -2473,9 +2462,6 @@ ena_down(struct ena_adapter *adapter) if (!ENA_FLAG_ISSET(ENA_FLAG_DEV_UP, adapter)) return; - /* Drain timer service to avoid admin queue race condition. */ - ENA_TIMER_DRAIN(adapter); - ena_log(adapter->pdev, INFO, "device is going DOWN\n"); ENA_FLAG_CLEAR_ATOMIC(ENA_FLAG_DEV_UP, adapter); @@ -2500,8 +2486,6 @@ ena_down(struct ena_adapter *adapter) ena_free_all_rx_resources(adapter); counter_u64_add(adapter->dev_stats.interface_down, 1); - - ENA_TIMER_RESET(adapter); } static uint32_t @@ -3279,24 +3263,7 @@ ena_timer_service(void *data) if ((adapter->eni_metrics_sample_interval != 0) && (++adapter->eni_metrics_sample_interval_cnt >= adapter->eni_metrics_sample_interval)) { - /* - * There is no race with other admin queue calls, as: - * - Timer service runs after attach function ends, so all - * configuration calls to the admin queue are finished. - * - Timer service is temporarily stopped when bringing - * the interface up or down. - * - After interface is up, the driver doesn't use (at least - * for now) other functions writing to the admin queue. - * - * It may change in the future, so in that situation, the lock - * will be needed. ENA_LOCK_*() cannot be used for that purpose, - * as callout ena_timer_service is protected by them. It could - * lead to the deadlock if callout_drain() would hold the lock - * before ena_copy_eni_metrics() was executed. It's advised to - * use separate lock in that situation which will be used only - * for the admin queue. - */ - (void)ena_copy_eni_metrics(adapter); + taskqueue_enqueue(adapter->metrics_tq, &adapter->metrics_task); adapter->eni_metrics_sample_interval_cnt = 0; } @@ -3503,6 +3470,16 @@ err: return (rc); } +static void +ena_metrics_task(void *arg, int pending) +{ + struct ena_adapter *adapter = (struct ena_adapter *)arg; + + ENA_LOCK_LOCK(); + (void)ena_copy_eni_metrics(adapter); + ENA_LOCK_UNLOCK(); +} + static void ena_reset_task(void *arg, int pending) { @@ -3723,6 +3700,13 @@ ena_attach(device_t pdev) taskqueue_start_threads(&adapter->reset_tq, 1, PI_NET, "%s rstq", device_get_nameunit(adapter->pdev)); + /* Initialize metrics task queue */ + TASK_INIT(&adapter->metrics_task, 0, ena_metrics_task, adapter); + adapter->metrics_tq = taskqueue_create("ena_metrics_enqueue", + M_WAITOK | M_ZERO, taskqueue_thread_enqueue, &adapter->metrics_tq); + taskqueue_start_threads(&adapter->metrics_tq, 1, PI_NET, + "%s metricsq", device_get_nameunit(adapter->pdev)); + /* Initialize statistics */ ena_alloc_counters((counter_u64_t *)&adapter->dev_stats, sizeof(struct ena_stats_dev)); @@ -3801,6 +3785,11 @@ ena_detach(device_t pdev) ENA_TIMER_DRAIN(adapter); ENA_LOCK_UNLOCK(); + /* Release metrics task */ + while (taskqueue_cancel(adapter->metrics_tq, &adapter->metrics_task, NULL)) + taskqueue_drain(adapter->metrics_tq, &adapter->metrics_task); + taskqueue_free(adapter->metrics_tq); + /* Release reset task */ while (taskqueue_cancel(adapter->reset_tq, &adapter->reset_task, NULL)) taskqueue_drain(adapter->reset_tq, &adapter->reset_task); diff --git a/sys/dev/ena/ena.h b/sys/dev/ena/ena.h index b100b091c89f..3219c4cf7455 100644 --- a/sys/dev/ena/ena.h +++ b/sys/dev/ena/ena.h @@ -469,6 +469,8 @@ struct ena_adapter { uint32_t next_monitored_tx_qid; struct task reset_task; struct taskqueue *reset_tq; + struct task metrics_task; + struct taskqueue *metrics_tq; int wd_active; sbintime_t keep_alive_timeout; sbintime_t missing_tx_timeout;