| Message ID | c65873a3-7394-4107-99a7-83f20030779c@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 127532cd0f060ebc3c4cbca81b6438728ad5896e |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] r8169: improve handling task scheduling | expand |
On 11/27/23 18:20, Heiner Kallweit wrote: > If we know that the task is going to be a no-op, don't even schedule it. > And remove the check for netif_running() in the worker function, the > check for flag RTL_FLAG_TASK_ENABLED is sufficient. Note that we can't > remove the check for flag RTL_FLAG_TASK_ENABLED in the worker function > because we have no guarantee when it will be executed. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/net/ethernet/realtek/r8169_main.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c > index b74d7cc50..91d9dc5a7 100644 > --- a/drivers/net/ethernet/realtek/r8169_main.c > +++ b/drivers/net/ethernet/realtek/r8169_main.c > @@ -2228,6 +2228,9 @@ u16 rtl8168h_2_get_adc_bias_ioffset(struct rtl8169_private *tp) > > static void rtl_schedule_task(struct rtl8169_private *tp, enum rtl_flag flag) > { > + if (!test_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags)) > + return; > + > set_bit(flag, tp->wk.flags); > schedule_work(&tp->wk.work); > } > @@ -4468,8 +4471,7 @@ static void rtl_task(struct work_struct *work) > > rtnl_lock(); > > - if (!netif_running(tp->dev) || > - !test_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags)) > + if (!test_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags)) > goto out_unlock; > > if (test_and_clear_bit(RTL_FLAG_TASK_TX_TIMEOUT, tp->wk.flags)) { sounds plausible and likely to run better, Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 27 Nov 2023 18:20:11 +0100 you wrote: > If we know that the task is going to be a no-op, don't even schedule it. > And remove the check for netif_running() in the worker function, the > check for flag RTL_FLAG_TASK_ENABLED is sufficient. Note that we can't > remove the check for flag RTL_FLAG_TASK_ENABLED in the worker function > because we have no guarantee when it will be executed. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > [...] Here is the summary with links: - [net-next] r8169: improve handling task scheduling https://git.kernel.org/netdev/net-next/c/127532cd0f06 You are awesome, thank you!
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c index b74d7cc50..91d9dc5a7 100644 --- a/drivers/net/ethernet/realtek/r8169_main.c +++ b/drivers/net/ethernet/realtek/r8169_main.c @@ -2228,6 +2228,9 @@ u16 rtl8168h_2_get_adc_bias_ioffset(struct rtl8169_private *tp) static void rtl_schedule_task(struct rtl8169_private *tp, enum rtl_flag flag) { + if (!test_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags)) + return; + set_bit(flag, tp->wk.flags); schedule_work(&tp->wk.work); } @@ -4468,8 +4471,7 @@ static void rtl_task(struct work_struct *work) rtnl_lock(); - if (!netif_running(tp->dev) || - !test_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags)) + if (!test_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags)) goto out_unlock; if (test_and_clear_bit(RTL_FLAG_TASK_TX_TIMEOUT, tp->wk.flags)) {
If we know that the task is going to be a no-op, don't even schedule it. And remove the check for netif_running() in the worker function, the check for flag RTL_FLAG_TASK_ENABLED is sufficient. Note that we can't remove the check for flag RTL_FLAG_TASK_ENABLED in the worker function because we have no guarantee when it will be executed. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/ethernet/realtek/r8169_main.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)