[src/trunk]: src/sys/dev/pci/ixgbe Fix race between ixgbe_msix_admin() and ix...

[knakahara <knakahara%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/b2d7b0482e17
branches:  trunk
changeset: 943655:b2d7b0482e17
user:      knakahara <knakahara%NetBSD.org@localhost>
date:      Mon Sep 07 09:14:53 2020 +0000

description:
Fix race between ixgbe_msix_admin() and ixgbe_handle_admin(), pointed out by ozaki-r@n.o.

The race is caused by the following.
CPU#A processes workqueue, CPU#B processes admin interrupt.
    (0) one of CPUs already calls ixgbe_schedule_admin_tasklet()
        such as ixgbe_handle_timer()
    (1) CPU#A: read adapter->task_requests
    (2) CPU#B: set adapter->task_requests
    (3) CPU#B: read(and try to set) adapter->admin_pending
               but adapter->admin_pending is set, so does not
               call workqueue_enqueue()
    (4) CPU#A: clear adapter->admin_pending
that is, the tasks set by (2) is not processed as missfire workqueue by (3).

diffstat:

 sys/dev/pci/ixgbe/ixgbe.c |  10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diffs (31 lines):

diff -r ba635c85c93e -r b2d7b0482e17 sys/dev/pci/ixgbe/ixgbe.c
--- a/sys/dev/pci/ixgbe/ixgbe.c Mon Sep 07 09:14:47 2020 +0000
+++ b/sys/dev/pci/ixgbe/ixgbe.c Mon Sep 07 09:14:53 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ixgbe.c,v 1.257 2020/09/07 05:50:58 msaitoh Exp $ */
+/* $NetBSD: ixgbe.c,v 1.258 2020/09/07 09:14:53 knakahara Exp $ */
 
 /******************************************************************************
 
@@ -4805,6 +4805,13 @@
         */
        IFNET_LOCK(ifp);
        IXGBE_CORE_LOCK(adapter);
+       /*
+        * Clear the admin_pending flag before reading task_requests to avoid
+        * missfiring workqueue though setting task_request.
+        * Hmm, ixgbe_schedule_admin_tasklet() can extra-fire though
+        * task_requests are done by prior workqueue, but it is harmless.
+        */
+       atomic_store_relaxed(&adapter->admin_pending, 0);
        while ((req =
                (adapter->task_requests & ~IXGBE_REQUEST_TASK_NEED_ACKINTR))
            != 0) {
@@ -4841,7 +4848,6 @@
                }
 #endif
        }
-       atomic_store_relaxed(&adapter->admin_pending, 0);
        if ((adapter->task_requests & IXGBE_REQUEST_TASK_NEED_ACKINTR) != 0) {
                atomic_and_32(&adapter->task_requests,
                    ~IXGBE_REQUEST_TASK_NEED_ACKINTR);