BPF List
 help / color / mirror / Atom feed
From: "Lin Ma" <linma@zju.edu.cn>
To: "Jakub Kicinski" <kuba@kernel.org>
Cc: jesse.brandeburg@intel.com, anthony.l.nguyen@intel.com,
	davem@davemloft.net, edumazet@google.com, pabeni@redhat.com,
	ast@kernel.org, daniel@iogearbox.net, hawk@kernel.org,
	john.fastabend@gmail.com, intel-wired-lan@lists.osuosl.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	bpf@vger.kernel.org
Subject: Re: [PATCH v0] idb: Add rtnl_lock to avoid data race
Date: Tue, 9 Aug 2022 11:05:19 +0800 (GMT+08:00)	[thread overview]
Message-ID: <35502986.7352e.18280905841.Coremail.linma@zju.edu.cn> (raw)
In-Reply-To: <20220808115511.5b574db2@kernel.org>

Hello there,

> 
> What about the disable path coming from sysfs? This looks incomplete to
> me. Perhaps take a look at commit 1e53834ce541 ("ixgbe: Add locking to
> prevent panic when setting sriov_numvfs to zero") for some inspiration.

Thanks for the advice, I sent the new version of the patch which uses a new spinlock to avoid race cases such as described in commit 1e53834ce541.

Additionally, I also keep the rtnl_lock to eliminate the races that come from netdev core. Although this can also be handled with the newly added spinlock, I found that adding the spinlock every time accessing the VF resources is not trivial.
(If you think that keep using the spinlock is better I will craft a new version of patch)

It seems that ixgbe_disable_sriov also suffers from the mentioned races from netdev core. If you think the rtnl_lock solution is fine, I will also send a patch for that driver too.

Thanks
Lin Ma

  reply	other threads:[~2022-08-09  3:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-08  8:10 [PATCH v0] idb: Add rtnl_lock to avoid data race Lin Ma
2022-08-08 18:55 ` Jakub Kicinski
2022-08-09  3:05   ` Lin Ma [this message]
2022-08-08 20:50 ` Edward Cree

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=35502986.7352e.18280905841.Coremail.linma@zju.edu.cn \
    --to=linma@zju.edu.cn \
    --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=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --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