Re: [PATCH iwl-next v3] e1000e: Fix real-time violations on link up

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/16/24 20:23, Gerhard Engleder wrote:
@@ -331,8 +331,15 @@ void e1000e_update_mc_addr_list_generic(struct e1000_hw *hw,
      }
      /* replace the entire MTA table */
-    for (i = hw->mac.mta_reg_count - 1; i >= 0; i--)
+    for (i = hw->mac.mta_reg_count - 1; i >= 0; i--) {
          E1000_WRITE_REG_ARRAY(hw, E1000_MTA, i, hw- >mac.mta_shadow[i]);
+
+        /* do not queue up too many posted writes to prevent increased
+         * latency for other devices on the interconnect
+         */
+        if ((i % 8) == 0 && i != 0)
+            e1e_flush();


I would prefer to avoid adding this code to all devices, particularly those that don't operate on real-time systems. Implementing this code will introduce three additional MMIO transactions which will increase the driver start time in various flows (up, probe, etc.).

Is there a specific reason not to use if (IS_ENABLED(CONFIG_PREEMPT_RT)) as Andrew initially suggested?

Andrew made two suggestions: IS_ENABLED(CONFIG_PREEMPT_RT) which I used
in the first version after the RFC. And he suggested to check for a
compromise between RT and none RT performance, as some distros might
enable PREEMPT_RT in the future.
Przemek suggested to remove the PREEMPT_RT check as "this change sounds
reasonable also for the standard kernel" after the first version with
IS_ENABLED(CONFIG_PREEMPT_RT).

I used the PREEMPT_RT dependency to limit effects to real-time systems,
to not make none real-time systems slower. But I could also follow the
reasoning of Andrew and Przemek. With that said, I have no problem to
add IS_ENABLED(CONFIG_PREEMPT_RT) again.

Gerhard

I'm also fine with limiting the change to RT kernels.




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux