linux-kernel - Re: [PATCH] e1000e: Assign DPM_FLAG_SMART_SUSPEND and DPM_FLAG_MAY_SKIP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20201201005725.GA32155@chenyu-office.sh.intel.com>
Date:   Tue, 1 Dec 2020 08:57:26 +0800
From:   Chen Yu <yu.c.chen@...el.com>
To:     Kai-Heng Feng <kai.heng.feng@...onical.com>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Len Brown <len.brown@...el.com>,
        Tony Nguyen <anthony.l.nguyen@...el.com>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        "moderated list:INTEL ETHERNET DRIVERS" 
        <intel-wired-lan@...ts.osuosl.org>,
        Linux PM list <linux-pm@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        Sasha Neftin <sasha.neftin@...el.com>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>
Subject: Re: [PATCH] e1000e: Assign DPM_FLAG_SMART_SUSPEND and
 DPM_FLAG_MAY_SKIP_RESUME to speed up s2ram

On Fri, Nov 27, 2020 at 08:20:17PM +0800, Kai-Heng Feng wrote:
> Thanks for the confirmation. How about a different approach? 
> Simply use direct-complete to let PM core handle the rest:
>
Thanks for your suggestion and sorry about replying too late.
Yes, using direct-complete could leverage pm core to do that,
although this is a little different than my original thought
to mainly skip the resume process. But anyway, I'll use direct
complete and send a v3 out.
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index b30f00891c03..1d1424a20733 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -25,6 +25,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/aer.h>
>  #include <linux/prefetch.h>
> +#include <linux/suspend.h>
>  
>  #include "e1000.h"
>  
> @@ -6868,6 +6869,20 @@ static void e1000e_disable_aspm_locked(struct pci_dev *pdev, u16 state)
>  	__e1000e_disable_aspm(pdev, state, 1);
>  }
>  
> +static int e1000e_pm_prepare(struct device *dev)
> +{
> +	return pm_runtime_suspended(dev) &&
> +	       pm_suspend_via_firmware() &&
> +	       !device_may_wakeup(dev);
> +}
device_may_wakeup() is not needed as pm core will check it anyway.
> +
> +static void e1000e_pm_complete(struct device *dev)
> +{
> +	/* Detect link change */
> +	if (pm_runtime_suspended(dev))
> +		pm_request_resume(dev);
> +}
There is no need to force resume the device, just keep it
runtime suspended would be okay. Besides the pm core's complete()
will restore runtime usage_count which is increased by pm core's
prepare(), so there is no need to do that here again.
> +
>  static int e1000e_pm_thaw(struct device *dev)
>  {
>  	struct net_device *netdev = dev_get_drvdata(dev);
> @@ -7665,9 +7680,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	e1000_print_device_info(adapter);
>  
> -	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
> -
DPM_FLAG_SMART_PREPARE must be set otherwise pci subsystem will ignore the
.prepare() from the driver.
> -	if (pci_dev_run_wake(pdev) && hw->mac.type < e1000_pch_cnp)
> +	if (pci_dev_run_wake(pdev))
>  		pm_runtime_put_noidle(&pdev->dev);
>
I would prefer to only disable runtime for cnp, in case of
any user regressions.

thanks,
Chenyu