Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1361440

Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM

Show all headers | View raw

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 20 Mar 2016 09:35:52 -0700

> On Sun, 2016-03-20 at 11:43 +0100, Geert Uytterhoeven wrote:
>> If CONFIG_PM=n:
>> 
>>     drivers/net/usb/lan78xx.c: In function ‘lan78xx_get_stats64’:
>>     drivers/net/usb/lan78xx.c:3274: error: ‘struct dev_pm_info’ has no member named ‘runtime_auto’
>> 
>> If PM is disabled, the runtime_auto flag is not available, but auto
>> suspend is not enabled anyway.  Hence protect the check for runtime_auto
>> by #ifdef CONFIG_PM to fix this.
>> 
>> Fixes: a59f8c5b048dc938 ("lan78xx: add ndo_get_stats64")
>> Reported-by: Guenter Roeck <linux@roeck-us.net>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>> Alternatively, we can add a dev_pm_runtime_auto_is_enabled() wrapper to
>> include/linux/pm.h, which always return false if CONFIG_PM is disabled.
>> 
>> The only other user in non-core code (drivers/usb/core/sysfs.c) has a
>> big #ifdef CONFIG_PM check around all PM-related code.
>> 
>> Thoughts?
>> ---
>>  drivers/net/usb/lan78xx.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
>> index d36d5ebf37f355f2..7b9ac47b2ecf9905 100644
>> --- a/drivers/net/usb/lan78xx.c
>> +++ b/drivers/net/usb/lan78xx.c
>> @@ -3271,7 +3271,9 @@ struct rtnl_link_stats64 *lan78xx_get_stats64(struct net_device *netdev,
>>  	 * periodic reading from HW will prevent from entering USB auto suspend.
>>  	 * if autosuspend is disabled, read from HW.
>>  	 */
>> +#ifdef CONFIG_PM
>>  	if (!dev->udev->dev.power.runtime_auto)
>> +#endif
>>  		lan78xx_update_stats(dev);
>>  
>>  	mutex_lock(&dev->stats.access_lock);
> 
> Note that a ndo_get_stat64() handler is not allowed to sleep,
> so the mutex_lock() is not wise...
> 
> Historically /proc/net/dev handler but also bonding ndo_get_stats() used
> RCU or a rwlock or a spinlock.
> 
> So a complete fix would need to get rid of this mutex as well.

This function is also buggy for another reason.  The driver needs to
capture the statistics when the runtime suspend happens.

Since it's much easier to find things wrong rather than right with
this new code, I've decided to completely revert the commit that added
lan78xx_get_stats64().

Once a correct version is implemented we can add it back.

Thanks.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Geert Uytterhoeven <geert@linux-m68k.org> - 2016-03-20 11:50 +0100
  Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Eric Dumazet <eric.dumazet@gmail.com> - 2016-03-20 17:40 +0100
    Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM David Miller <davem@davemloft.net> - 2016-03-20 22:00 +0100
  Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Guenter Roeck <linux@roeck-us.net> - 2016-03-21 00:00 +0100
    RE: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM <Woojung.Huh@microchip.com> - 2016-03-21 16:00 +0100
  Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Oliver Neukum <oneukum@suse.com> - 2016-03-21 09:40 +0100
    RE: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM <Woojung.Huh@microchip.com> - 2016-03-21 15:50 +0100
    Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Alan Stern <stern@rowland.harvard.edu> - 2016-03-21 16:00 +0100
      Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Oliver Neukum <oneukum@suse.com> - 2016-03-21 18:40 +0100
        Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Alan Stern <stern@rowland.harvard.edu> - 2016-03-21 19:30 +0100
          RE: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM <Woojung.Huh@microchip.com> - 2016-03-21 20:20 +0100
            RE: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Alan Stern <stern@rowland.harvard.edu> - 2016-03-21 20:30 +0100
              RE: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM <Woojung.Huh@microchip.com> - 2016-03-21 21:10 +0100
                RE: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Alan Stern <stern@rowland.harvard.edu> - 2016-03-21 22:10 +0100
          Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Oliver Neukum <oneukum@suse.com> - 2016-03-21 20:30 +0100
            Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Alan Stern <stern@rowland.harvard.edu> - 2016-03-21 20:40 +0100
              Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Oliver Neukum <oneukum@suse.com> - 2016-03-22 11:00 +0100
                Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Alan Stern <stern@rowland.harvard.edu> - 2016-03-22 15:30 +0100
                Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Oliver Neukum <oneukum@suse.com> - 2016-03-22 15:40 +0100
                Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Alan Stern <stern@rowland.harvard.edu> - 2016-03-22 16:20 +0100
                Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM Oliver Neukum <oneukum@suse.com> - 2016-03-22 16:40 +0100

csiph-web