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


Groups > linux.kernel > #1296658 > unrolled thread

RE: [PATCH v2] r8152: fix lockup when runtime PM is enabled

Started byHayes Wang <hayeswang@realtek.com>
First post2015-12-22 10:50 +0100
Last post2015-12-24 17:10 +0100
Articles 12 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  RE: [PATCH v2] r8152: fix lockup when runtime PM is enabled Hayes Wang <hayeswang@realtek.com> - 2015-12-22 10:50 +0100
    Re: [PATCH v2] r8152: fix lockup when runtime PM is enabled Oliver Neukum <oneukum@suse.com> - 2015-12-22 12:10 +0100
      RE: [PATCH v2] r8152: fix lockup when runtime PM is enabled Hayes Wang <hayeswang@realtek.com> - 2015-12-23 04:40 +0100
        Re: [PATCH v2] r8152: fix lockup when runtime PM is enabled Oliver Neukum <oneukum@suse.de> - 2015-12-23 09:30 +0100
          RE: [PATCH v2] r8152: fix lockup when runtime PM is enabled Hayes Wang <hayeswang@realtek.com> - 2015-12-23 10:30 +0100
            Re: [PATCH v2] r8152: fix lockup when runtime PM is enabled Oliver Neukum <oneukum@suse.com> - 2015-12-23 11:50 +0100
              RE: [PATCH v2] r8152: fix lockup when runtime PM is enabled Hayes Wang <hayeswang@realtek.com> - 2015-12-23 12:20 +0100
            RE: [PATCH v2] r8152: fix lockup when runtime PM is enabled Alan Stern <stern@rowland.harvard.edu> - 2015-12-24 02:40 +0100
              Re: [PATCH v2] r8152: fix lockup when runtime PM is enabled Oliver Neukum <oneukum@suse.com> - 2015-12-24 08:20 +0100
                Re: [PATCH v2] r8152: fix lockup when runtime PM is enabled Alan Stern <stern@rowland.harvard.edu> - 2015-12-24 16:20 +0100
                  Re: [PATCH v2] r8152: fix lockup when runtime PM is enabled Oliver Neukum <oneukum@suse.com> - 2015-12-24 16:50 +0100
                    Re: [PATCH v2] r8152: fix lockup when runtime PM is enabled Alan Stern <stern@rowland.harvard.edu> - 2015-12-24 17:10 +0100

#1296658 — RE: [PATCH v2] r8152: fix lockup when runtime PM is enabled

FromHayes Wang <hayeswang@realtek.com>
Date2015-12-22 10:50 +0100
SubjectRE: [PATCH v2] r8152: fix lockup when runtime PM is enabled
Message-ID<qIrJT-2dB-1@gated-at.bofh.it>
 Peter Wu [mailto:peter@lekensteyn.nl]
> Sent: Tuesday, December 08, 2015 10:33 PM
[...]
> I found another problem with runtime PM. When a device is suspended via
> autosuspend and a system suspend takes place, there is no network I/O
> after resume. Triggering a renegotiation (ethtool -r eth1) brings back
> network activity.

I think it is relative to the firmware. Could you try the driver from Realtek website?

Best Regards,
Hayes

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1296722

FromOliver Neukum <oneukum@suse.com>
Date2015-12-22 12:10 +0100
Message-ID<qIsZj-3ag-5@gated-at.bofh.it>
In reply to#1296658
On Tue, 2015-12-22 at 09:48 +0000, Hayes Wang wrote:
>  Peter Wu [mailto:peter@lekensteyn.nl]
> > Sent: Tuesday, December 08, 2015 10:33 PM
> [...]
> > I found another problem with runtime PM. When a device is suspended via
> > autosuspend and a system suspend takes place, there is no network I/O
> > after resume. Triggering a renegotiation (ethtool -r eth1) brings back
> > network activity.
> 
> I think it is relative to the firmware. Could you try the driver from Realtek website?

Hi,

at the risk of repeating myself I must say that there is a logic flaw
in the driver. If you look at this code:

static int rtl8152_resume(struct usb_interface *intf)
{
        struct r8152 *tp = usb_get_intfdata(intf);

        mutex_lock(&tp->control);

        if (!test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
                tp->rtl_ops.init(tp);
                netif_device_attach(tp->netdev);
        }

        if (netif_running(tp->netdev) && tp->netdev->flags & IFF_UP) {
                if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
                        rtl_runtime_suspend_enable(tp, false);
                        clear_bit(SELECTIVE_SUSPEND, &tp->flags);
                        napi_disable(&tp->napi);
                        set_bit(WORK_ENABLE, &tp->flags);
                        if (netif_carrier_ok(tp->netdev))
                                rtl_start_rx(tp);
                        napi_enable(&tp->napi);
                } else {
                        tp->rtl_ops.up(tp);
                        rtl8152_set_speed(tp, AUTONEG_ENABLE,
                                          tp->mii.supports_gmii ?
                                          SPEED_1000 : SPEED_100,
                                          DUPLEX_FULL);
                        netif_carrier_off(tp->netdev);
                        set_bit(WORK_ENABLE, &tp->flags);
                }

You need to understand that its use of the flag SELECTIVE_SUSPEND
is invalid. SELECTIVE_SUSPEND is used at two places in the driver.

Once in rtl8152_start_xmit(), where it is working but misnamed.
At that time you need to know whether the device is suspended.
To the device it does not matter whether the suspension is selective
or for the whole bus. It cannot tell. The driver just needs to know
whether it should resume the device if a packet to be transmitted
through it is given to the driver. So far all is well.

But at the time rtl8152_resume() is called the flag has become
meaningless. It tells you whether the device was selectively suspended
(we call that autosuspended, but the concept is the same), but you
take it to mean that it was _only_ selectively suspended. It does not
tell you that.
That matters a lot because the behavior of the host regarding powering
the bus during S3 and S4 is not defined. As I mentioned the device
can't tell whether it is selectively suspended. But it does notice
if its power supply is cut. In that case the driver must reinitialize
the device.
The current code does that conditional on
!test_bit(SELECTIVE_SUSPEND ... )
That is wrong because you need to do this if power was lost. The test
only tells you whether the device was selectively suspend before
power was lost (if power was lost). That is not the same thing at all.

The way the USB subsystem is designed is that it tells you whether power
had been cut by calling reset_resume() if power was cut or resume() if
power was kept.
If reset_resume() is called you must always execute this code:

                tp->rtl_ops.init(tp);

and

                        tp->rtl_ops.up(tp);
                        rtl8152_set_speed(tp, AUTONEG_ENABLE,
                                          tp->mii.supports_gmii ?
                                          SPEED_1000 : SPEED_100,
                                          DUPLEX_FULL);

The conditions used in rtl8152_resume() are wrong.
That is the reason "ethtool -r eth1" is reported to restore the device.
It triggers equivalent operations.

It is clear to me that you cannot get away with using the same operation
for resume() and reset_resume() in your driver. It is fundamentally
impossible. Firmware cannot fix it.

Sorry for the length of the explanation.

	HTH
		Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1297233

FromHayes Wang <hayeswang@realtek.com>
Date2015-12-23 04:40 +0100
Message-ID<qIIro-4oC-5@gated-at.bofh.it>
In reply to#1296722
T2xpdmVyIE5ldWt1bSBbbWFpbHRvOm9uZXVrdW1Ac3VzZS5jb21dDQpbLi4uXQ0KPiBJdCBpcyBj
bGVhciB0byBtZSB0aGF0IHlvdSBjYW5ub3QgZ2V0IGF3YXkgd2l0aCB1c2luZyB0aGUgc2FtZSBv
cGVyYXRpb24NCj4gZm9yIHJlc3VtZSgpIGFuZCByZXNldF9yZXN1bWUoKSBpbiB5b3VyIGRyaXZl
ci4gSXQgaXMgZnVuZGFtZW50YWxseQ0KPiBpbXBvc3NpYmxlLiBGaXJtd2FyZSBjYW5ub3QgZml4
IGl0Lg0KDQpJIHdvdWxkIHRoaW5rIGhvdyB0byBmaXggaXQuDQoNCj4gU29ycnkgZm9yIHRoZSBs
ZW5ndGggb2YgdGhlIGV4cGxhbmF0aW9uLg0KDQpUaGFua3MgZm9yIHlvdXIgcmVzcG9uc2UuIEkg
aGF2ZSBzb21lIHF1ZXN0aW9ucy4gV2hhdCBhcmUgdGhlIGZsb3dzIHdoZW4NCnRoZSBzeXN0ZW0g
cmVzdW1lIGZvbGxvd3MgYSBzeXN0ZW0gc3VzcGVuZCB3aGljaCBmb2xsb3dzIGEgYXV0b3N1c3Bl
bmQ/DQpBcmUgdGhleSBhcyBmb2xsb3dpbmc/DQoNCjEuIHN1c3BlbmQoKSB3aXRoIFBNU0dfSVNf
QVVUTyBmb3IgYXV0b3N1c3BuZWQuDQoyLiBzdXNwZW5kKCkgZm9yIHN5c3RlbSBzdXNwZW5kLg0K
My4gcmVzdW1lKCkgZm9yIHN5c3RlbSByZXN1bWUuDQoNCkFuZCwgc2hvdWxkIHRoZSBkZXZpY2Ug
ZXhpc3QgYXV0b3N1c3BlbmQgYmVmb3JlICgyKT8gDQoNCkJlc3QgUmVnYXJkcywNCkhheWVzDQoN
Cg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1297272

FromOliver Neukum <oneukum@suse.de>
Date2015-12-23 09:30 +0100
Message-ID<qIMY2-7ed-9@gated-at.bofh.it>
In reply to#1297233
On Wed, 2015-12-23 at 03:31 +0000, Hayes Wang wrote:
> Oliver Neukum [mailto:oneukum@suse.com]
> [...]
> > It is clear to me that you cannot get away with using the same operation
> > for resume() and reset_resume() in your driver. It is fundamentally
> > impossible. Firmware cannot fix it.
> 
> I would think how to fix it.
> 
> > Sorry for the length of the explanation.
> 
> Thanks for your response. I have some questions. What are the flows when
> the system resume follows a system suspend which follows a autosuspend?
> Are they as following?
> 
> 1. suspend() with PMSG_IS_AUTO for autosuspned.
> 2. suspend() for system suspend.
> 3. resume() for system resume.
> 
> And, should the device exist autosuspend before (2)? 

No, step (2) does not exist. Calls to suspend() and [reset_]resume()
always balance. Usually a driver shouldn't care about system suspend.
The way the driver is currently coded will also fail for Port-Power Off.

	Regards
		Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1297287

FromHayes Wang <hayeswang@realtek.com>
Date2015-12-23 10:30 +0100
Message-ID<qINU6-7NH-7@gated-at.bofh.it>
In reply to#1297272
T2xpdmVyIE5ldWt1bSBbbWFpbHRvOm9uZXVrdW1Ac3VzZS5kZV0NCj4gU2VudDogV2VkbmVzZGF5
LCBEZWNlbWJlciAyMywgMjAxNSA0OjIwIFBNDQpbLi4uXQ0KPiBObywgc3RlcCAoMikgZG9lcyBu
b3QgZXhpc3QuIENhbGxzIHRvIHN1c3BlbmQoKSBhbmQgW3Jlc2V0X11yZXN1bWUoKQ0KPiBhbHdh
eXMgYmFsYW5jZS4gVXN1YWxseSBhIGRyaXZlciBzaG91bGRuJ3QgY2FyZSBhYm91dCBzeXN0ZW0g
c3VzcGVuZC4NCj4gVGhlIHdheSB0aGUgZHJpdmVyIGlzIGN1cnJlbnRseSBjb2RlZCB3aWxsIGFs
c28gZmFpbCBmb3IgUG9ydC1Qb3dlciBPZmYuDQoNCkl0IGlzIGRpZmZlcmVudCB3aXRoIFdpbmRv
d3MuIFRoZSBXaW5kb3dzIHdvdWxkIHJlc3VtZSB0aGUgZGV2aWNlIGJlZm9yZQ0Kc3lzdGVtIHN1
c3BlbmQsIGlmIHRoZSBzeXN0ZW0gc3VzcGVuZCBmb2xsb3dzIHRoZSBhdXRvc3VzcGVuZC4NCg0K
V291bGQgdGhpcyBiZSBhIHByb2JsZW0/IEFmdGVyIHN5c3RlbSBzdXNwZW5kLCB0aGUgZGV2aWNl
IG1heSB3YWtlIHVwDQp0aGUgc3lzdGVtIHdoZW4gcmVjZWl2aW5nIGFueSBwYWNrZXQsIG5vdCBv
bmx5IG1hZ2ljIHBhY2tldC4gVGhlIHdha2UNCmV2ZW50cyBhcmUgZGlmZmVyZW50IGZvciBzeXN0
ZW0gc3VzcGVuZCBhbmQgYXV0b3N1c3BlbmQuIEhvd2V2ZXIsIEkNCmNvdWxkbid0IGNoYW5nZSB0
aGUgd2FrZSBldmVudCwgYmVjYXVzZSB0aGUgYXV0b3N1c3BlbmQgb2NjdXJzIGZpcnN0LA0KYW5k
IHRoZSBzdXNwZW5kKCkgaXMgb25seSBjYWxsZWQgb25jZS4NCg0KQmVzdCBSZWdhcmRzLA0KSGF5
ZXMNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1297335

FromOliver Neukum <oneukum@suse.com>
Date2015-12-23 11:50 +0100
Message-ID<qIP9w-8u4-15@gated-at.bofh.it>
In reply to#1297287
On Wed, 2015-12-23 at 09:20 +0000, Hayes Wang wrote:
> Oliver Neukum [mailto:oneukum@suse.de]
> > Sent: Wednesday, December 23, 2015 4:20 PM
> [...]
> > No, step (2) does not exist. Calls to suspend() and [reset_]resume()
> > always balance. Usually a driver shouldn't care about system suspend.
> > The way the driver is currently coded will also fail for Port-Power Off.
> 
> It is different with Windows. The Windows would resume the device before
> system suspend, if the system suspend follows the autosuspend.
> 
> Would this be a problem? After system suspend, the device may wake up
> the system when receiving any packet, not only magic packet. The wake
> events are different for system suspend and autosuspend. However, I
> couldn't change the wake event, because the autosuspend occurs first,
> and the suspend() is only called once.

That is indeed a problem and I need to think a bit about finding
a good solution. If you are happy with an inelegant solution, you can
use a pm_notifier, which will tell you that the system is going
to suspend. This is documented:

https://www.kernel.org/doc/Documentation/power/notifiers.txt

	HTH
		Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1297369

FromHayes Wang <hayeswang@realtek.com>
Date2015-12-23 12:20 +0100
Message-ID<qIPCx-s6-11@gated-at.bofh.it>
In reply to#1297335
T2xpdmVyIE5ldWt1bSBbbWFpbHRvOm9uZXVrdW1Ac3VzZS5jb21dDQo+IFNlbnQ6IFdlZG5lc2Rh
eSwgRGVjZW1iZXIgMjMsIDIwMTUgNjo0NiBQTQ0KWy4uLl0NCj4gVGhhdCBpcyBpbmRlZWQgYSBw
cm9ibGVtIGFuZCBJIG5lZWQgdG8gdGhpbmsgYSBiaXQgYWJvdXQgZmluZGluZw0KPiBhIGdvb2Qg
c29sdXRpb24uIElmIHlvdSBhcmUgaGFwcHkgd2l0aCBhbiBpbmVsZWdhbnQgc29sdXRpb24sIHlv
dSBjYW4NCj4gdXNlIGEgcG1fbm90aWZpZXIsIHdoaWNoIHdpbGwgdGVsbCB5b3UgdGhhdCB0aGUg
c3lzdGVtIGlzIGdvaW5nDQo+IHRvIHN1c3BlbmQuIFRoaXMgaXMgZG9jdW1lbnRlZDoNCj4gDQo+
IGh0dHBzOi8vd3d3Lmtlcm5lbC5vcmcvZG9jL0RvY3VtZW50YXRpb24vcG93ZXIvbm90aWZpZXJz
LnR4dA0KDQpUaGFua3MuIEkgd291bGQgc3R1ZHkgaXQgYWZ0ZXIgZml4aW5nIHRoZSByZXNldF9y
ZXN1bWUoKSBpc3N1ZS4NCg0KQmVzdCBSZWdhcmRzLA0KSGF5ZXMNCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1297714

FromAlan Stern <stern@rowland.harvard.edu>
Date2015-12-24 02:40 +0100
Message-ID<qJ32O-kI-19@gated-at.bofh.it>
In reply to#1297287
On Wed, 23 Dec 2015, Hayes Wang wrote:

> Oliver Neukum [mailto:oneukum@suse.de]
> > Sent: Wednesday, December 23, 2015 4:20 PM
> [...]
> > No, step (2) does not exist. Calls to suspend() and [reset_]resume()
> > always balance. Usually a driver shouldn't care about system suspend.
> > The way the driver is currently coded will also fail for Port-Power Off.
> 
> It is different with Windows. The Windows would resume the device before
> system suspend, if the system suspend follows the autosuspend.
> 
> Would this be a problem? After system suspend, the device may wake up
> the system when receiving any packet, not only magic packet. The wake
> events are different for system suspend and autosuspend. However, I
> couldn't change the wake event, because the autosuspend occurs first,
> and the suspend() is only called once.

I don't understand why the wakeup conditions are different.  It seems
to me that the choice of which packets will generate a wakeup ought to
depend on the user's selection, not on the kind of suspend.  For
instance, if the user says that only a magic packet should cause a
wakeup then that should be true for both runtime suspend and system
suspend.

To put it another way, as far as the device is concerned a suspend is
just a suspend -- there's no different between a runtime suspend and a
system suspend.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1297773

FromOliver Neukum <oneukum@suse.com>
Date2015-12-24 08:20 +0100
Message-ID<qJ8lP-3K9-1@gated-at.bofh.it>
In reply to#1297714
On Wed, 2015-12-23 at 20:32 -0500, Alan Stern wrote:

> I don't understand why the wakeup conditions are different.  It seems
> to me that the choice of which packets will generate a wakeup ought to
> depend on the user's selection, not on the kind of suspend.  For
> instance, if the user says that only a magic packet should cause a
> wakeup then that should be true for both runtime suspend and system
> suspend.
> 
> To put it another way, as far as the device is concerned a suspend is
> just a suspend -- there's no different between a runtime suspend and a
> system suspend.

This literally true, but the host and the driver care.
If we autosuspend a running network device, any packet
(maybe filtered for MAC) should cause a remote wake up,
else we'd lose packets.

But you cannot keep that setting if the system goes down
or any broadcast packet would resume the whole system.
Yet you cannot just disable remote wake up, as WoL packages
still must trigger a remote wake up.
So there are drivers which must change settings on devices
as the system goes to sleep, even if their devices have
already been autosuspended. We could use the notifier chains
for that. But can this solution be called elegant?

	Merry Christmas
		Oliver



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1297895

FromAlan Stern <stern@rowland.harvard.edu>
Date2015-12-24 16:20 +0100
Message-ID<qJfQl-8ob-1@gated-at.bofh.it>
In reply to#1297773
On Thu, 24 Dec 2015, Oliver Neukum wrote:

> On Wed, 2015-12-23 at 20:32 -0500, Alan Stern wrote:
> 
> > I don't understand why the wakeup conditions are different.  It seems
> > to me that the choice of which packets will generate a wakeup ought to
> > depend on the user's selection, not on the kind of suspend.  For
> > instance, if the user says that only a magic packet should cause a
> > wakeup then that should be true for both runtime suspend and system
> > suspend.
> > 
> > To put it another way, as far as the device is concerned a suspend is
> > just a suspend -- there's no different between a runtime suspend and a
> > system suspend.
> 
> This literally true, but the host and the driver care.
> If we autosuspend a running network device, any packet
> (maybe filtered for MAC) should cause a remote wake up,
> else we'd lose packets.

That's also true during system suspend.

> But you cannot keep that setting if the system goes down
> or any broadcast packet would resume the whole system.
> Yet you cannot just disable remote wake up, as WoL packages
> still must trigger a remote wake up.

This means that sometimes you want to avoid losing packets and other 
times you do want to lose packets.  That is a policy decision, and 
therefore it should be made by the user, not the kernel.

> So there are drivers which must change settings on devices
> as the system goes to sleep, even if their devices have
> already been autosuspended. We could use the notifier chains
> for that. But can this solution be called elegant?

Instead of the driver trying to do this automatically, you could rely 
on userspace telling the driver which packets should cause a wakeup.  
The setting could be updated immediately before and after each system 
suspend.

I admit this is more awkward than having the driver make a choice based 
on the type of suspend.  This is a case where the resources provided by 
the PM core aren't adequate for what the driver needs.  The PM core 
distinguishes between wakeup enabled or disabled; it doesn't 
distinguish among different levels of wakekup.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1297912

FromOliver Neukum <oneukum@suse.com>
Date2015-12-24 16:50 +0100
Message-ID<qJgjo-6R-21@gated-at.bofh.it>
In reply to#1297895
On Thu, 2015-12-24 at 10:14 -0500, Alan Stern wrote:
> On Thu, 24 Dec 2015, Oliver Neukum wrote:
> 
> > On Wed, 2015-12-23 at 20:32 -0500, Alan Stern wrote:

> > But you cannot keep that setting if the system goes down
> > or any broadcast packet would resume the whole system.
> > Yet you cannot just disable remote wake up, as WoL packages
> > still must trigger a remote wake up.
> 
> This means that sometimes you want to avoid losing packets and other 
> times you do want to lose packets.  That is a policy decision, and 
> therefore it should be made by the user, not the kernel.

Indeed it is and there is a tool for this with a defined
interface called "ethtool"
The problem here is not the policy decision, but implementing
it in kernel space.

> > So there are drivers which must change settings on devices
> > as the system goes to sleep, even if their devices have
> > already been autosuspended. We could use the notifier chains
> > for that. But can this solution be called elegant?
> 
> Instead of the driver trying to do this automatically, you could rely 
> on userspace telling the driver which packets should cause a wakeup.

It does.

> The setting could be updated immediately before and after each system 
> suspend.

The API is so that user space sets the policy, which persists until
user space changes the setting and the kernel implements it. The
problem is that to do so the kernel needs to do IO to the device
as the system is about to suspend.
Thus the driver may need to resume the device and it needs to learn
that the system is about to go to sleep, even if the device it
manages is already autosuspended.

> I admit this is more awkward than having the driver make a choice based 
> on the type of suspend.  This is a case where the resources provided by 

It also is a race condition, unless you want user space to disable
autosuspend as the system is about to go to sleep. And it makes
relatively little sense, as enabling remote wakeup is the last thing
we do before the device suspends. Setting the filters long before that
doesn't make much sense.

> the PM core aren't adequate for what the driver needs.  The PM core 
> distinguishes between wakeup enabled or disabled; it doesn't 
> distinguish among different levels of wakekup.

True and sanely it cannot. We could only distinguish between drivers
which need their devices to be resumed before the system suspends and
the rest.
Or we tell driver coders to use the notifier chains.

	Regards
		Oliver



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1297966

FromAlan Stern <stern@rowland.harvard.edu>
Date2015-12-24 17:10 +0100
Message-ID<qJgCK-u9-7@gated-at.bofh.it>
In reply to#1297912
On Thu, 24 Dec 2015, Oliver Neukum wrote:

> On Thu, 2015-12-24 at 10:14 -0500, Alan Stern wrote:
> > On Thu, 24 Dec 2015, Oliver Neukum wrote:
> > 
> > > On Wed, 2015-12-23 at 20:32 -0500, Alan Stern wrote:
> 
> > > But you cannot keep that setting if the system goes down
> > > or any broadcast packet would resume the whole system.
> > > Yet you cannot just disable remote wake up, as WoL packages
> > > still must trigger a remote wake up.
> > 
> > This means that sometimes you want to avoid losing packets and other 
> > times you do want to lose packets.  That is a policy decision, and 
> > therefore it should be made by the user, not the kernel.
> 
> Indeed it is and there is a tool for this with a defined
> interface called "ethtool"

No; ethtool affects the wakeup setting for system suspend, but not
for runtime suspend.  I was referring to something that would specify 
the setting for both cases.  But perhaps that doesn't make sense, 
because you never want to drop relevant packets during runtime suspend.  
If you did, you would run "ifconfig down" instead.

> > the PM core aren't adequate for what the driver needs.  The PM core 
> > distinguishes between wakeup enabled or disabled; it doesn't 
> > distinguish among different levels of wakekup.
> 
> True and sanely it cannot. We could only distinguish between drivers
> which need their devices to be resumed before the system suspends and
> the rest.
> Or we tell driver coders to use the notifier chains.

"Resume before system suspend" sounds like a reasonable thing to
implement, for devices that have multiple levels of wakeup settings.  
Would you like to post a proposal on linux-pm for this?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web