All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>
To: Jakub Kicinski <kuba@kernel.org>,
	Tony Nguyen <anthony.l.nguyen@intel.com>
Cc: davem@davemloft.net, pabeni@redhat.com, edumazet@google.com,
	netdev@vger.kernel.org, Ngai-Mint Kwan <ngai-mint.kwan@intel.com>,
	Mateusz Polchlopek <mateusz.polchlopek@intel.com>,
	Pawel Chmielewski <pawel.chmielewski@intel.com>,
	Simon Horman <horms@kernel.org>,
	Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com>,
	larysa.zaremba@intel.com
Subject: Re: [PATCH net] ice: Do not get coalesce settings while in reset
Date: Fri, 17 May 2024 15:31:58 +0200	[thread overview]
Message-ID: <24d3e9ed-0e56-40d6-b53f-e379855aa740@linux.intel.com> (raw)
In-Reply-To: <382a4740-cc05-4897-94e3-aac4f12b2300@linux.intel.com>

On 06.05.2024 15:30, Dawid Osuchowski wrote:
> On 02.05.2024 04:56, Jakub Kicinski wrote:
>> Did you not add locks around reset to allow waiting instead of returning
>> -EBUSY to user space? I feel like we've been over this...
> 
> Will use the approach with ice_wait_for_reset() in next revision, thanks
> 
> --Dawid

Hey Jakub,

I went ahead with the approach of using ice_wait_for_reset() [1], 
however this resulted in a new problem in the reset flow. I want to 
prove why I think returning immediately with -EBUSY (or perhaps -EAGAIN) 
is the correct way in this particular case.

The issue has to deal with the way both the ethtool handler and the 
adapter reset flow call rtnl_lock() during operation. If we wait for 
reset completion inside of an ethtool handling function such as 
ice_get_coalesce(), the wait will always timeout due to reset being 
blocked by rtnl_lock() inside of ice_queue_set_napi() (which is called 
during reset process), and in turn we will always return -EBUSY anyways, 
with the added hang time of the timeout value (in case of [1] it's 10 
seconds).

There are other places where similar deadlock can occur, not only in 
ice_queue_set_napi() and Larysa is currently working on an extensive 
solution to this problem.

--Dawid

[1] 
https://lore.kernel.org/netdev/20240506153307.114104-1-dawid.osuchowski@linux.intel.com/

      reply	other threads:[~2024-05-17 13:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-30 18:14 [PATCH net] ice: Do not get coalesce settings while in reset Tony Nguyen
2024-05-02  2:56 ` Jakub Kicinski
2024-05-06 13:30   ` Dawid Osuchowski
2024-05-17 13:31     ` Dawid Osuchowski [this message]

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=24d3e9ed-0e56-40d6-b53f-e379855aa740@linux.intel.com \
    --to=dawid.osuchowski@linux.intel.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=himasekharx.reddy.pucha@intel.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=larysa.zaremba@intel.com \
    --cc=mateusz.polchlopek@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=ngai-mint.kwan@intel.com \
    --cc=pabeni@redhat.com \
    --cc=pawel.chmielewski@intel.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.