Date:	Tue, 25 Sep 2012 01:10:31 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Bjørn Mork <bjorn@...k.no>
Cc:	Oliver Neukum <oneukum@...e.de>,
	"David S. Miller" <davem@...emloft.net>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Fink Dmitry <finik@...com>, Rafael Wysocki <rjw@...k.pl>,
	Alan Stern <stern@...land.harvard.edu>, netdev@...r.kernel.org,
	linux-usb@...r.kernel.org
Subject: Re: [RFC PATCH 0/3] usbnet: support runtime PM triggered by link change

On Thu, Sep 20, 2012 at 4:30 PM, Bjørn Mork <bjorn@...k.no> wrote:
> Ming Lei <ming.lei@...onical.com> writes:
>> On Mon, Sep 17, 2012 at 4:04 PM, Oliver Neukum <oneukum@...e.de> wrote:
>>
>>> 1) Does it actually save power? You are constantly waking up a CPU.
>>
>> Of course, it does. I don't know it will save how much power just on
>> usbnet device, but it may save power from usb hub and usb host
>> controller in the bus at least.
>>
>> Anyway we don't need to waste power if the link of usbnet is down.
>>
>> It just wake up CPU one time each 3sec. In modern linux distribution,
>> the CPU will be wakup tens times per second, so it shouldn't be a
>> big problem.
>
> I do not buy that constantly polling a device necessarily saves any
> power compared to keeping the USB link to the device alive.  You need to
> measure the savings if you want us to believe that.

Yes, good suggestion.

>
> You are not only waking the host CPU.  You are also waking the device
> CPU.

Some usbnet devices doesn't have CPU, and I don't think there are
one in smsc95xx or asix, :-)

>
> Any usbnet device will consist of more than one building block, having
> at least a network block, a USB block and a CPU.  For all you know, the
> device CPU might be in deep sleep until it has to service either the
> network or USB block, and those might also be sleeping until they see an
> interrupt.  Constantly polling the device to receive network link status
> might use considerably more power than keeping the USB link up waiting
> for a link notification.
>
> If you were to implement this feature anyway, then I would prefer a
> userspace based approach instead.  I see no reason to keep the timer in
> the kernel.  You could decide to suspend whenever the netdev is down,
> and only poll the link when userspace tries to bring up the netdev.

I am wondering if userspace knows when it should bring up the interface,
maybe it depends on the link becomes up, maybe udev always bring it up.

> That would let a userspace daemon implement the same feature by trying
> to bring up the netdev every 3 seconds (or whatever frequency the user
> selected).  And it would allow me to avoid the polling until I know I
> have plugged in a cable.

I don't see any advantage by using a userspace timer, and it will convert to
a kernel timer finally.

Also putting the interface down may introduce some side effect on user
space: the IP address may be lost, etc.

>
>>> From that perspective it is possible that leaving on the ethernet is actually
>>> better in terms of power. Only measurements can answer that question.
>>
>> You know it is a bit difficult to test power save for this situation. And
>> most of runtime PM patches didn't provide power save data. In fact,
>> I'd like to do it, but I have not some equipment to measure it, :-(.
>
> We don't know, you don't know.  But you claim that your change saves
> power, so please provide some documentation showing that it does.

The motivation of the patch is that suspending the usbnet device may put
all usb devices in the bus into suspend.

I am trying to get some data by powertop, but looks it is not reliable enough.

I even found that the discharge rate of battery when the asix is disconnected
and let the whole bus suspend is even more than when the asix is connected
with ethernet cable link down and all devices in the whole usb bus are active.
There is about 1w difference, so odd.

I will try to get some correct power data.

>
>>> 2) Do we have many devices that would be serviced with this approach?
>>
>> At least I found asix can be serviced by this approach. Considered asix
>> is quite popular, it is worthy of the effort. Also the below devices can be
>> serviced by the patch in theory:
>>
>>                    dm9601.c / mcs7830.c / sierra_net.c
>
> The sierra_net.c driver is only used for wireless devices AFAIK. I
> really don't see the use case for network link based resume of that.
> There is no cable to plug.  Userspace will have to initiate a
> connection.

Wireless link is still one kind of link, :-)

Otherwise, why sierra_net.c bother to use netif_carrier_on?

>
> And the DirectIP device I've got (MC7710) supports remote wakeup.  I
> assume that will be the case for all such devices, given that this is
> mostly a firmware feature. So the correct fix for sierra_net.c is to add
> support for remote wakeup.  Then you will be able to suspend the device
> regardless of whether the link is up or down, without the constant
> polling.

In fact, the patch supports link change via remote wakeup, and it may be
useful if remote wakeup is not supported by incoming packet.

Not mention the power save data, the patch is not mature enough, for example,
ethtool_ops interface may access a suspended device.

I will update it if some progresses are got.

Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists