All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Larysa Zaremba <larysa.zaremba@intel.com>
Cc: <intel-wired-lan@lists.osuosl.org>,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Alexei Starovoitov <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	John Fastabend <john.fastabend@gmail.com>,
	Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<bpf@vger.kernel.org>, <magnus.karlsson@intel.com>,
	Michal Kubiak <michal.kubiak@intel.com>
Subject: Re: [PATCH iwl-net 0/3] ice: fix synchronization between .ndo_bpf() and reset
Date: Wed, 12 Jun 2024 14:09:35 -0700	[thread overview]
Message-ID: <20240612140935.54981c49@kernel.org> (raw)
In-Reply-To: <ZmlGppe04yuGHvPx@lzaremba-mobl.ger.corp.intel.com>

On Wed, 12 Jun 2024 08:56:38 +0200 Larysa Zaremba wrote:
> On Tue, Jun 11, 2024 at 07:38:37PM -0700, Jakub Kicinski wrote:
> > On Mon, 10 Jun 2024 17:37:12 +0200 Larysa Zaremba wrote:  
> > > Fix the problems that are triggered by tx_timeout and ice_xdp() calls,
> > > including both pool and program operations.  
> > 
> > Is there really no way for ice to fix the locking? :(
> > The busy loops and trylocks() are not great, and seem like duct tape.
> 
> The locking mechanisms I use here do not look pretty, but if I am not missing 
> anything, the synchronization they provide must be robust.

Robust as in they may be correct here, but you lose lockdep and all
other infra normal mutex would give you.

> A prettier way of protecting the same critical sections would be replacing 
> ICE_CFG_BUSY around ice_vsi_rebuild() with rtnl_lock(), this would eliminate 
> locking code from .ndo_bpf() altogether, ice_rebuild_pending() logic will have 
> to stay.
> 
> At some point I have decided to avoid using rtnl_lock(), if I do not have to. I 
> think this is a goal worth pursuing?

Is the reset for failure recovery, rather than reconfiguration? 
If so netif_device_detach() is generally the best way of avoiding
getting called (I think I mentioned it to someone @intal recently).

WARNING: multiple messages have this Message-ID (diff)
From: Jakub Kicinski <kuba@kernel.org>
To: Larysa Zaremba <larysa.zaremba@intel.com>
Cc: Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	netdev@vger.kernel.org, John Fastabend <john.fastabend@gmail.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	Michal Kubiak <michal.kubiak@intel.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	magnus.karlsson@intel.com, intel-wired-lan@lists.osuosl.org,
	bpf@vger.kernel.org, Paolo Abeni <pabeni@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	linux-kernel@vger.kernel.org
Subject: Re: [Intel-wired-lan] [PATCH iwl-net 0/3] ice: fix synchronization between .ndo_bpf() and reset
Date: Wed, 12 Jun 2024 14:09:35 -0700	[thread overview]
Message-ID: <20240612140935.54981c49@kernel.org> (raw)
In-Reply-To: <ZmlGppe04yuGHvPx@lzaremba-mobl.ger.corp.intel.com>

On Wed, 12 Jun 2024 08:56:38 +0200 Larysa Zaremba wrote:
> On Tue, Jun 11, 2024 at 07:38:37PM -0700, Jakub Kicinski wrote:
> > On Mon, 10 Jun 2024 17:37:12 +0200 Larysa Zaremba wrote:  
> > > Fix the problems that are triggered by tx_timeout and ice_xdp() calls,
> > > including both pool and program operations.  
> > 
> > Is there really no way for ice to fix the locking? :(
> > The busy loops and trylocks() are not great, and seem like duct tape.
> 
> The locking mechanisms I use here do not look pretty, but if I am not missing 
> anything, the synchronization they provide must be robust.

Robust as in they may be correct here, but you lose lockdep and all
other infra normal mutex would give you.

> A prettier way of protecting the same critical sections would be replacing 
> ICE_CFG_BUSY around ice_vsi_rebuild() with rtnl_lock(), this would eliminate 
> locking code from .ndo_bpf() altogether, ice_rebuild_pending() logic will have 
> to stay.
> 
> At some point I have decided to avoid using rtnl_lock(), if I do not have to. I 
> think this is a goal worth pursuing?

Is the reset for failure recovery, rather than reconfiguration? 
If so netif_device_detach() is generally the best way of avoiding
getting called (I think I mentioned it to someone @intal recently).

  reply	other threads:[~2024-06-12 21:09 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-10 15:37 [PATCH iwl-net 0/3] ice: fix synchronization between .ndo_bpf() and reset Larysa Zaremba
2024-06-10 15:37 ` [Intel-wired-lan] " Larysa Zaremba
2024-06-10 15:37 ` [PATCH iwl-net 1/3] ice: synchronize XDP setup with reset Larysa Zaremba
2024-06-10 15:37   ` [Intel-wired-lan] " Larysa Zaremba
2024-06-10 15:37 ` [PATCH iwl-net 2/3] ice: fix locking in ice_xsk_pool_setup() Larysa Zaremba
2024-06-10 15:37   ` [Intel-wired-lan] " Larysa Zaremba
2024-06-10 15:37 ` [PATCH iwl-net 3/3] ice: make NAPI setting code aware that rtnl-locked request is waiting Larysa Zaremba
2024-06-10 15:37   ` [Intel-wired-lan] " Larysa Zaremba
2024-06-12  2:38 ` [PATCH iwl-net 0/3] ice: fix synchronization between .ndo_bpf() and reset Jakub Kicinski
2024-06-12  2:38   ` [Intel-wired-lan] " Jakub Kicinski
2024-06-12  6:56   ` Larysa Zaremba
2024-06-12  6:56     ` [Intel-wired-lan] " Larysa Zaremba
2024-06-12 21:09     ` Jakub Kicinski [this message]
2024-06-12 21:09       ` Jakub Kicinski
2024-06-13  8:54       ` Larysa Zaremba
2024-06-13  8:54         ` [Intel-wired-lan] " Larysa Zaremba
2024-06-13 10:44         ` Przemek Kitszel
2024-06-13 10:44           ` Przemek Kitszel
2024-06-13 14:13         ` Jakub Kicinski
2024-06-13 14:13           ` [Intel-wired-lan] " Jakub Kicinski
2024-06-13 15:36           ` Larysa Zaremba
2024-06-13 15:36             ` [Intel-wired-lan] " Larysa Zaremba
2024-06-13 15:40             ` Jakub Kicinski
2024-06-13 15:40               ` [Intel-wired-lan] " Jakub Kicinski

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=20240612140935.54981c49@kernel.org \
    --to=kuba@kernel.org \
    --cc=anthony.l.nguyen@intel.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hawk@kernel.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jesse.brandeburg@intel.com \
    --cc=john.fastabend@gmail.com \
    --cc=larysa.zaremba@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=magnus.karlsson@intel.com \
    --cc=michal.kubiak@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 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.