From nobody Tue Jul 26 19:30:55 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 4Lsn7z40hxz4WwkD; Tue, 26 Jul 2022 19:30:55 +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 4Lsn7z3R2Hz42fN; Tue, 26 Jul 2022 19:30:55 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1658863855; 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=+X5SNPur+i2/OUI4Gy1HjbeckULq5EgDAUkLkIhQ8pA=; b=ExIWIXbByjRjodUIRiIfflomciGe+hK6hGXxUJa1ig3lBjW//yokJ/HZs/0BmJfBao8mAr uSPtDri5OaCiLkPBoiPkENidz8CdCqc7qj/M4r+onq7IV7UaOx9cSUzM/Zq4DQkCI+dZSr H4fS6lA+BH8nVNKOu7KPaNyuPvUiM4M9MTk3hrU94hvsa3tIv9xZxuVFTrheiYe+4xfeNN zXvpBdB7SUMnsUgm7SyTu8GSn3LINe3arRiZ2sPuc9IT2AIN1D8Y7hr5N3+KlFJrtVBDEd GqXTq7uab8X6NN686MsZQ/TGQkMxHQNinUCtyiY0+WYT8YR1qLLm/bZhuQ8pvw== 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 4Lsn7z2WdvzpYW; Tue, 26 Jul 2022 19:30:55 +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 26QJUtIh029686; Tue, 26 Jul 2022 19:30:55 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 26QJUtE2029685; Tue, 26 Jul 2022 19:30:55 GMT (envelope-from git) Date: Tue, 26 Jul 2022 19:30:55 GMT Message-Id: <202207261930.26QJUtE2029685@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: aea2edcc00a1 - stable/13 - 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/13 X-Git-Reftype: branch X-Git-Commit: aea2edcc00a1e94e99334f6fe7dec6d472ecb632 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1658863855; 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=+X5SNPur+i2/OUI4Gy1HjbeckULq5EgDAUkLkIhQ8pA=; b=SDE5v9l4xwlx7ASLTurMFpymHliPiPvoLOCJeCyrtf+Xy2FwMvTLrRbbAjXO7fclvmwiy+ MZuY3nNwwMuvNYqciy44SVJ/HIdY/XmbPHWbJnNMXNcDONDFfOj8qFaFGUks07h+7Lf5Eq zeFFA8Jo+W7yhjXDwnGRujbOUUXcNml1qgdFNYJw9r5QDTO3cPAYF1dREL0OBzzth/nad5 0W3/X4SvQXRmmdZmnPyya1LzGGLOvo6qES8o2JdSLqr6w98+BjbaqsuMd+u5hpLtT/cPPQ xHKlT2/EAkepiqH+5fYm6zBYAWpr50aYwQ5IHmrf36h/fgfrAiZcVmmcw7UWLA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1658863855; a=rsa-sha256; cv=none; b=Fo1KPfiWppX354BTrhgbCFQv0nRG3KJfgZys1pNYRqIanH0xkjoAhLEk3RgtNjNY3tgrWB UvhhXGBfDfTpUZMZGhQsmo20VMjsVlrap78sRJDl1OIx/IzatZslYurFHkXTDYZwUIOjee GbwDmNgIL+gX9wAlRhDkWjLDIXl4HwMPkcOeoqcfpcds0xZ6bInBi22NWbqgkHOX/FeH08 J4JxtkqBhqRkgrNDuXjLiQnpL9SGNHPydMedBOlo1uKGxt3pDN+2K4yNPaO4vkNzdy2DG/ 20H1UqeLfQ5INYTzECYmF40GCIln3pMj/lzuELeCE/Sl+JFz3BENWWWtougBOw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by mw: URL: https://cgit.FreeBSD.org/src/commit/?id=aea2edcc00a1e94e99334f6fe7dec6d472ecb632 commit aea2edcc00a1e94e99334f6fe7dec6d472ecb632 Author: Dawid Gorecki AuthorDate: 2022-06-10 09:18:10 +0000 Commit: Marcin Wojtas CommitDate: 2022-07-26 19:30:15 +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 343f2f9626cd..6a3956ddd181 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); } @@ -2474,9 +2463,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); @@ -2501,8 +2487,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 @@ -3275,24 +3259,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; } @@ -3499,6 +3466,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) { @@ -3719,6 +3696,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)); @@ -3797,6 +3781,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 56e14bd800ff..43fbaad17b78 100644 --- a/sys/dev/ena/ena.h +++ b/sys/dev/ena/ena.h @@ -470,6 +470,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;