RE: [net-next 07/10] igb: Added rcu_lock to avoid race

On Wed, 2013-06-26 at 16:42 +0000, Abodunrin, Akeem G wrote:

quoted

-----Original Message-----
From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
Sent: Wednesday, June 26, 2013 4:20 AM
To: Kirsher, Jeffrey T
Cc: davem@davemloft.net; Abodunrin, Akeem G; netdev@vger.kernel.org;
gospo@redhat.com; sassmann@redhat.com
Subject: Re: [net-next 07/10] igb: Added rcu_lock to avoid race

On Wed, 2013-06-26 at 03:55 -0700, Jeff Kirsher wrote:

quoted

From: "Akeem G. Abodunrin" <redacted>

This patch adds rcu_lock to avoid possible race condition with
igb_update_stats function accessing the rings in free_ q_vector.

Signed-off-by: Akeem G Abodunrin <redacted>
Tested-by: Aaron Brown <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c

b/drivers/net/ethernet/intel/igb/igb_main.c
index 7112e52..c74ad78 100644

--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c

@@ -705,6 +705,8 @@ static void __exit igb_exit_module(void)
 	dca_unregister_notify(&dca_notifier);
 #endif
 	pci_unregister_driver(&igb_driver);
+
+	rcu_barrier(); /* Wait for completion of call_rcu()'s */
 }

This is not needed.

If this is really needed, a better comment would be welcomed ;)

Thanks Eric!

I believe we need this to make sure memory is released before removing
driver code from the kernel, especially with each call to rcu - that
is the reason for the comment "Wait for completion of call_rcu()'s".

You did not add call_rcu() in this patch, therefore you do not need
this.

I am extra careful of people adding useless rcu synchronization just
because they feel better :)