From: Jakub Kicinski <kuba@kernel.org>
To: Jacob Keller <jacob.e.keller@intel.com>
Cc: Kees Cook <keescook@chromium.org>,
Leon Romanovsky <leon@kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"Ruhl, Michael J" <michael.j.ruhl@intel.com>,
"edumazet@google.com" <edumazet@google.com>,
"intel-wired-lan@lists.osuosl.org"
<intel-wired-lan@lists.osuosl.org>,
"pabeni@redhat.com" <pabeni@redhat.com>,
"davem@davemloft.net" <davem@davemloft.net>
Subject: Re: [Intel-wired-lan] [PATCH net-next 5/6] igb: Do not free q_vector unless new one was allocated
Date: Mon, 7 Nov 2022 19:55:26 -0800 [thread overview]
Message-ID: <20221107195526.5ef1262e@kernel.org> (raw)
In-Reply-To: <c051fa25-6047-0efb-7049-be08f566d1fb@intel.com>
On Mon, 7 Nov 2022 10:35:14 -0800 Jacob Keller wrote:
> > I understand the issue what you are trying to solve, I just don't
> > understand your RCU code. I would expect calls to rcu_dereference()
> > in order to get q_vector and rcu_assign_pointer() to clear
> > adapter->q_vector[v_idx], but igb has none.
>
> the uses of kfree_rcu were introduced by 5536d2102a2d ("igb: Combine
> q_vector and ring allocation into a single function")
>
> The commit doesn't mention switching from kfree to kfree_rcu and I
> suspect that the igb driver is not actually really using RCU semantics
> properly.
>
> The closest explanation is that the get_stats64 function might be
> accessing the ring and thus needs the RCU grace period.. but I think
> you're right in that we're missing the necessary RCU access macros.
Alright, expecting a follow up for this.
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
WARNING: multiple messages have this Message-ID (diff)
From: Jakub Kicinski <kuba@kernel.org>
To: Jacob Keller <jacob.e.keller@intel.com>
Cc: Leon Romanovsky <leon@kernel.org>,
"Ruhl, Michael J" <michael.j.ruhl@intel.com>,
"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"pabeni@redhat.com" <pabeni@redhat.com>,
"edumazet@google.com" <edumazet@google.com>,
Kees Cook <keescook@chromium.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"Brandeburg, Jesse" <jesse.brandeburg@intel.com>,
"intel-wired-lan@lists.osuosl.org"
<intel-wired-lan@lists.osuosl.org>,
"G, GurucharanX" <gurucharanx.g@intel.com>
Subject: Re: [PATCH net-next 5/6] igb: Do not free q_vector unless new one was allocated
Date: Mon, 7 Nov 2022 19:55:26 -0800 [thread overview]
Message-ID: <20221107195526.5ef1262e@kernel.org> (raw)
In-Reply-To: <c051fa25-6047-0efb-7049-be08f566d1fb@intel.com>
On Mon, 7 Nov 2022 10:35:14 -0800 Jacob Keller wrote:
> > I understand the issue what you are trying to solve, I just don't
> > understand your RCU code. I would expect calls to rcu_dereference()
> > in order to get q_vector and rcu_assign_pointer() to clear
> > adapter->q_vector[v_idx], but igb has none.
>
> the uses of kfree_rcu were introduced by 5536d2102a2d ("igb: Combine
> q_vector and ring allocation into a single function")
>
> The commit doesn't mention switching from kfree to kfree_rcu and I
> suspect that the igb driver is not actually really using RCU semantics
> properly.
>
> The closest explanation is that the get_stats64 function might be
> accessing the ring and thus needs the RCU grace period.. but I think
> you're right in that we're missing the necessary RCU access macros.
Alright, expecting a follow up for this.
next prev parent reply other threads:[~2022-11-08 3:55 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-04 20:54 [PATCH net-next 0/6][pull request] Intel Wired LAN Driver Updates 2022-11-04 (ixgbe, ixgbevf, igb) Tony Nguyen
2022-11-04 20:54 ` [PATCH net-next 1/6] ixgbe: change MAX_RXD/MAX_TXD based on adapter type Tony Nguyen
2022-11-04 20:54 ` [PATCH net-next 2/6] ixgbe: Remove local variable Tony Nguyen
2022-11-04 20:54 ` [PATCH net-next 3/6] ixgbe: Remove unneeded semicolon Tony Nguyen
2022-11-04 20:54 ` [PATCH net-next 4/6] ixgbevf: Add error messages on vlan error Tony Nguyen
2022-11-04 20:54 ` [Intel-wired-lan] [PATCH net-next 5/6] igb: Do not free q_vector unless new one was allocated Tony Nguyen
2022-11-04 20:54 ` Tony Nguyen
2022-11-07 7:03 ` [Intel-wired-lan] " Leon Romanovsky
2022-11-07 7:03 ` Leon Romanovsky
2022-11-07 13:55 ` [Intel-wired-lan] " Ruhl, Michael J
2022-11-07 13:55 ` Ruhl, Michael J
2022-11-07 17:45 ` [Intel-wired-lan] " Leon Romanovsky
2022-11-07 17:45 ` Leon Romanovsky
2022-11-07 18:35 ` [Intel-wired-lan] " Jacob Keller
2022-11-07 18:35 ` Jacob Keller
2022-11-08 3:55 ` Jakub Kicinski [this message]
2022-11-08 3:55 ` Jakub Kicinski
2022-11-04 20:54 ` [Intel-wired-lan] [PATCH net-next 6/6] igb: Proactively round up to kmalloc bucket size Tony Nguyen
2022-11-04 20:54 ` Tony Nguyen
2022-11-07 7:05 ` [PATCH net-next 0/6][pull request] Intel Wired LAN Driver Updates 2022-11-04 (ixgbe, ixgbevf, igb) Leon Romanovsky
2022-11-08 4:00 ` 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=20221107195526.5ef1262e@kernel.org \
--to=kuba@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jacob.e.keller@intel.com \
--cc=keescook@chromium.org \
--cc=leon@kernel.org \
--cc=michael.j.ruhl@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.