From: "Nelson, Shannon" <shannon.nelson@amd.com>
To: Jacob Keller <jacob.e.keller@intel.com>,
andrew+netdev@lunn.ch, brett.creeley@amd.com,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, michal.swiatkowski@linux.intel.com,
horms@kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
Subject: Re: [PATCH v3 net 1/4] pds_core: Prevent possible adminq overflow/stuck condition
Date: Wed, 16 Apr 2025 13:49:31 -0700 [thread overview]
Message-ID: <ac965836-ee8f-4b0f-8a69-8a76aec7835f@amd.com> (raw)
In-Reply-To: <61b952ee-d4e4-4e1a-bee6-4bde45ec1025@intel.com>
On 4/16/2025 1:13 PM, Jacob Keller wrote:
>
> On 4/15/2025 4:29 PM, Shannon Nelson wrote:
>> From: Brett Creeley <brett.creeley@amd.com>
>>
>> The pds_core's adminq is protected by the adminq_lock, which prevents
>> more than 1 command to be posted onto it at any one time. This makes it
>> so the client drivers cannot simultaneously post adminq commands.
>> However, the completions happen in a different context, which means
>> multiple adminq commands can be posted sequentially and all waiting
>> on completion.
>>
>> On the FW side, the backing adminq request queue is only 16 entries
>> long and the retry mechanism and/or overflow/stuck prevention is
>> lacking. This can cause the adminq to get stuck, so commands are no
>> longer processed and completions are no longer sent by the FW.
>>
>> As an initial fix, prevent more than 16 outstanding adminq commands so
>> there's no way to cause the adminq from getting stuck. This works
>> because the backing adminq request queue will never have more than 16
>> pending adminq commands, so it will never overflow. This is done by
>> reducing the adminq depth to 16.
>>
>
> What happens if a client driver tries to enqueue a request when the
> adminq is full? Does it just block until there is space, presumably
> holding the adminq_lock the entire time to prevent someone else from
> inserting?
Right now we will return -ENOSPC and it is up to the client to decide
whether or not it wants to do a retry.
We have another patch that has pdsc_adminq_post() doing a limited retry
loop which was part of the original posting [1], but Kuba suggested
using a semaphore instead. That sent us down a redesign branch that we
haven't been able to spend time on. We'd like to have kept the retry
loop patch until then to at least mitigate the situation, but the
discussion got dropped.
sln
[1]
https://lore.kernel.org/netdev/20250129004337.36898-3-shannon.nelson@amd.com/
next prev parent reply other threads:[~2025-04-16 20:49 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-15 23:29 [PATCH v3 net 0/4] pds_core: updates and fixes Shannon Nelson
2025-04-15 23:29 ` [PATCH v3 net 1/4] pds_core: Prevent possible adminq overflow/stuck condition Shannon Nelson
2025-04-16 20:13 ` Jacob Keller
2025-04-16 20:49 ` Nelson, Shannon [this message]
2025-04-16 23:34 ` Jacob Keller
2025-04-15 23:29 ` [PATCH v3 net 2/4] pds_core: handle unsupported PDS_CORE_CMD_FW_CONTROL result Shannon Nelson
2025-04-16 20:14 ` Jacob Keller
2025-04-15 23:29 ` [PATCH v3 net 3/4] pds_core: Remove unnecessary check in pds_client_adminq_cmd() Shannon Nelson
2025-04-16 20:15 ` Jacob Keller
2025-04-15 23:29 ` [PATCH v3 net 4/4] pds_core: make wait_context part of q_info Shannon Nelson
2025-04-16 20:16 ` Jacob Keller
2025-04-17 15:21 ` Jakub Kicinski
2025-04-18 22:31 ` Nelson, Shannon
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=ac965836-ee8f-4b0f-8a69-8a76aec7835f@amd.com \
--to=shannon.nelson@amd.com \
--cc=andrew+netdev@lunn.ch \
--cc=brett.creeley@amd.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=jacob.e.keller@intel.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=michal.swiatkowski@linux.intel.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox