From nobody Thu Jun 30 16:15:37 2022 X-Original-To: dev-commits-src-main@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 A3A638A396A; Thu, 30 Jun 2022 16:15:38 +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 4LYk2f00pKz3QlB; Thu, 30 Jun 2022 16:15:37 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1656605738; 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=WjEa6v2qGiNETqcDpw7G3xe4aK6Glai5vIfQ2sn+7X0=; b=JNvFR8gdMYKhEKVxcNrs/2r1u9PkmWJqYXtR1mTzQDtVX65lW1inbbU201ke4Lv5wat3DV y5mExaAOX8mvLY6TxGdgtK5Sd4MX8NWTevCVoVUaNtrUF6XStOVwjzRQCV5B/Dd9GSTQCK 37ty1W9k+n0K2gI6Xmv9wBXg3Tpcp+VEOshXXscVFAgkHFmMpc5vab4e+A9QhzOyg7rIKv xxF9x7l1ZyojUBhlRuW5QlZJyfF7A4WJ47k/2eCKiYFbz2WOiBarHsoi50hCv5nXUQTqWT cN86FCASKkNs0NHgjX/yEalZLDi/ZQRnl9inDbnHkkfg1gGk5nFZgTDKke8EpA== 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 A43C414EE; Thu, 30 Jun 2022 16:15:37 +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 25UGFba4014847; Thu, 30 Jun 2022 16:15:37 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 25UGFb3H014846; Thu, 30 Jun 2022 16:15:37 GMT (envelope-from git) Date: Thu, 30 Jun 2022 16:15:37 GMT Message-Id: <202206301615.25UGFb3H014846@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Marcin Wojtas Subject: git: b899a02ad733 - main - ena: Move ena_copy_eni_metrics into separate task List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@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/main X-Git-Reftype: branch X-Git-Commit: b899a02ad7330cae3c9bb08ad7975601dc3b9551 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1656605738; 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=WjEa6v2qGiNETqcDpw7G3xe4aK6Glai5vIfQ2sn+7X0=; b=xUyEltTBNzH1IxUigf0UL0vKtrrKE5JME2Xn9oTn254xx8/6qLceFFqL4a/EbIkTdIINpM I68Na59KTzfYntyLPQX1jgLCc8HIsars1Pg4HgLJiela1of8rNW4gsgXrdSshZHiZNH8Wo Ju7RJII2XTShJSPsOAicxfHLY3FKPqZzv3jcuwIFZVUkm0h5o0y/PIJHp3je9k+9LbsB4d 3c4MtlR4mmj877LCHTiZNE0PzvEJtPGESnS2MaKHX0JU4lxewsIFo3dHkq2UNkltIIs/D6 DAWrcMpMqqWVRHIE/mnXQUiIur2cSKbdHqTJHpD0oPYMe43HQKIUL4cwrPb9wA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1656605738; a=rsa-sha256; cv=none; b=Zxa0xzfhJ9I4GyEVklDA+QA6Qqb5yH7EEk7gtIDcXNoJE+cdGxcfi9tzwXcEzQGT85wE83 6LTct3tNMI39iDHLftOSfx4rUIStCoqmiLE3gnGD22+sXuuaIOUzuQRX9EY4tQOR6cIJgx VHyCYqIXmTuxUqd/vyjEmMdG4OpnGNyW4ytN7jvHtuA5u87OK0fVmOXFF7FdCbqMK9uivU fJ4VWninSCXsJKMgi/rxmu/fZEu4uQtgEFhmAR/1SIp695p9+FL9tshYil5LTDtSLzckGB 3cOgs0G1sQpOrTZYPiWh+MXtG+1TCSDHqCKYOGcRxhUO8n0iHL7+Q5YIITaMHA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by mw: URL: https://cgit.FreeBSD.org/src/commit/?id=b899a02ad7330cae3c9bb08ad7975601dc3b9551 commit b899a02ad7330cae3c9bb08ad7975601dc3b9551 Author: Dawid Gorecki AuthorDate: 2022-06-10 09:18:10 +0000 Commit: Marcin Wojtas CommitDate: 2022-06-30 15:31:53 +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. --- 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 ab9bb177ee25..e5854dc4d140 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;