From: "Nelson, Shannon" <shannon.nelson@amd.com>
To: darinzon@amazon.com, David Miller <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
netdev@vger.kernel.org
Cc: "Woodhouse, David" <dwmw@amazon.com>,
"Machulsky, Zorik" <zorik@amazon.com>,
"Matushevsky, Alexander" <matua@amazon.com>,
Saeed Bshara <saeedb@amazon.com>, "Wilson, Matt" <msw@amazon.com>,
"Liguori, Anthony" <aliguori@amazon.com>,
"Bshara, Nafea" <nafea@amazon.com>,
"Belgazal, Netanel" <netanel@amazon.com>,
"Saidi, Ali" <alisaidi@amazon.com>,
"Herrenschmidt, Benjamin" <benh@amazon.com>,
"Kiyanovski, Arthur" <akiyano@amazon.com>,
"Dagan, Noam" <ndagan@amazon.com>,
"Agroskin, Shay" <shayagr@amazon.com>,
"Itzko, Shahar" <itzko@amazon.com>,
"Abboud, Osama" <osamaabb@amazon.com>,
"Ostrovsky, Evgeny" <evostrov@amazon.com>,
"Tabachnik, Ofir" <ofirt@amazon.com>,
Netanel Belgazal <netanel@annapurnalabs.com>,
Sameeh Jubran <sameehj@amazon.com>,
Amit Bernstein <amitbern@amazon.com>
Subject: Re: [PATCH v1 net 2/4] net: ena: Wrong missing IO completions check order
Date: Wed, 10 Apr 2024 14:51:32 -0700 [thread overview]
Message-ID: <e4ff2cce-904b-45fb-8bf9-4ebfd14271c4@amd.com> (raw)
In-Reply-To: <20240410091358.16289-3-darinzon@amazon.com>
On 4/10/2024 2:13 AM, darinzon@amazon.com wrote:
> From: David Arinzon <darinzon@amazon.com>
>
> Missing IO completions check is called every second (HZ jiffies).
> This commit fixes several issues with this check:
>
> 1. Duplicate queues check:
> Max of 4 queues are scanned on each check due to monitor budget.
> Once reaching the budget, this check exits under the assumption that
> the next check will continue to scan the remainder of the queues,
> but in practice, next check will first scan the last already scanned
> queue which is not necessary and may cause the full queue scan to
> last a couple of seconds longer.
> The fix is to start every check with the next queue to scan.
> For example, on 8 IO queues:
> Bug: [0,1,2,3], [3,4,5,6], [6,7]
> Fix: [0,1,2,3], [4,5,6,7]
>
> 2. Unbalanced queues check:
> In case the number of active IO queues is not a multiple of budget,
> there will be checks which don't utilize the full budget
> because the full scan exits when reaching the last queue id.
> The fix is to run every TX completion check with exact queue budget
> regardless of the queue id.
> For example, on 7 IO queues:
> Bug: [0,1,2,3], [4,5,6], [0,1,2,3]
> Fix: [0,1,2,3], [4,5,6,0], [1,2,3,4]
> The budget may be lowered in case the number of IO queues is less
> than the budget (4) to make sure there are no duplicate queues on
> the same check.
> For example, on 3 IO queues:
> Bug: [0,1,2,0], [1,2,0,1]
> Fix: [0,1,2], [0,1,2]
>
> Fixes: 1738cd3ed342 ("net: ena: Add a driver for Amazon Elastic Network Adapters (ENA)")
> Signed-off-by: Amit Bernstein <amitbern@amazon.com>
> Signed-off-by: David Arinzon <darinzon@amazon.com>
Reviewed-by: Shannon Nelson <shannon.nelson@amd.com>
> ---
> drivers/net/ethernet/amazon/ena/ena_netdev.c | 21 +++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 09e7da1a..59befc0f 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -3481,10 +3481,11 @@ static void check_for_missing_completions(struct ena_adapter *adapter)
> {
> struct ena_ring *tx_ring;
> struct ena_ring *rx_ring;
> - int i, budget, rc;
> + int qid, budget, rc;
> int io_queue_count;
>
> io_queue_count = adapter->xdp_num_queues + adapter->num_io_queues;
> +
> /* Make sure the driver doesn't turn the device in other process */
> smp_rmb();
>
> @@ -3497,27 +3498,29 @@ static void check_for_missing_completions(struct ena_adapter *adapter)
> if (adapter->missing_tx_completion_to == ENA_HW_HINTS_NO_TIMEOUT)
> return;
>
> - budget = ENA_MONITORED_TX_QUEUES;
> + budget = min_t(u32, io_queue_count, ENA_MONITORED_TX_QUEUES);
>
> - for (i = adapter->last_monitored_tx_qid; i < io_queue_count; i++) {
> - tx_ring = &adapter->tx_ring[i];
> - rx_ring = &adapter->rx_ring[i];
> + qid = adapter->last_monitored_tx_qid;
> +
> + while (budget) {
> + qid = (qid + 1) % io_queue_count;
> +
> + tx_ring = &adapter->tx_ring[qid];
> + rx_ring = &adapter->rx_ring[qid];
>
> rc = check_missing_comp_in_tx_queue(adapter, tx_ring);
> if (unlikely(rc))
> return;
>
> - rc = !ENA_IS_XDP_INDEX(adapter, i) ?
> + rc = !ENA_IS_XDP_INDEX(adapter, qid) ?
> check_for_rx_interrupt_queue(adapter, rx_ring) : 0;
> if (unlikely(rc))
> return;
>
> budget--;
> - if (!budget)
> - break;
> }
>
> - adapter->last_monitored_tx_qid = i % io_queue_count;
> + adapter->last_monitored_tx_qid = qid;
> }
>
> /* trigger napi schedule after 2 consecutive detections */
> --
> 2.40.1
>
>
next prev parent reply other threads:[~2024-04-10 21:51 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-10 9:13 [PATCH v1 net 0/4] ENA driver bug fixes darinzon
2024-04-10 9:13 ` [PATCH v1 net 1/4] net: ena: Fix potential sign extension issue darinzon
2024-04-10 21:49 ` Nelson, Shannon
2024-04-10 9:13 ` [PATCH v1 net 2/4] net: ena: Wrong missing IO completions check order darinzon
2024-04-10 21:51 ` Nelson, Shannon [this message]
2024-04-10 9:13 ` [PATCH v1 net 3/4] net: ena: Fix incorrect descriptor free behavior darinzon
2024-04-10 21:52 ` Nelson, Shannon
2024-04-10 9:13 ` [PATCH v1 net 4/4] net: ena: Set tx_info->xdpf value to NULL darinzon
2024-04-10 21:53 ` Nelson, Shannon
2024-04-11 9:40 ` [PATCH v1 net 0/4] ENA driver bug fixes patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e4ff2cce-904b-45fb-8bf9-4ebfd14271c4@amd.com \
--to=shannon.nelson@amd.com \
--cc=akiyano@amazon.com \
--cc=aliguori@amazon.com \
--cc=alisaidi@amazon.com \
--cc=amitbern@amazon.com \
--cc=benh@amazon.com \
--cc=darinzon@amazon.com \
--cc=davem@davemloft.net \
--cc=dwmw@amazon.com \
--cc=evostrov@amazon.com \
--cc=itzko@amazon.com \
--cc=kuba@kernel.org \
--cc=matua@amazon.com \
--cc=msw@amazon.com \
--cc=nafea@amazon.com \
--cc=ndagan@amazon.com \
--cc=netanel@amazon.com \
--cc=netanel@annapurnalabs.com \
--cc=netdev@vger.kernel.org \
--cc=ofirt@amazon.com \
--cc=osamaabb@amazon.com \
--cc=saeedb@amazon.com \
--cc=sameehj@amazon.com \
--cc=shayagr@amazon.com \
--cc=zorik@amazon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.