mbox series

[net-next,0/7] Polling be gone on LAN95xx

Message ID cover.1651037513.git.lukas@wunner.de (mailing list archive)
Headers show
Series Polling be gone on LAN95xx | expand

Message

Lukas Wunner April 27, 2022, 5:48 a.m. UTC
Do away with link status polling on LAN95XX USB Ethernet
and rely on interrupts instead, thereby reducing bus traffic,
CPU overhead and improving interface bringup latency.

The meat of the series is in patch [5/7].  The preceding and
following patches are various cleanups to prepare for and
adjust to interrupt-driven link state detection.

Please review and test.  Thanks!

Lukas Wunner (7):
  usbnet: Run unregister_netdev() before unbind() again
  usbnet: smsc95xx: Don't clear read-only PHY interrupt
  usbnet: smsc95xx: Don't reset PHY behind PHY driver's back
  usbnet: smsc95xx: Avoid link settings race on interrupt reception
  usbnet: smsc95xx: Forward PHY interrupts to PHY driver to avoid
    polling
  net: phy: smsc: Cache interrupt mask
  net: phy: smsc: Cope with hot-removal in interrupt handler

 drivers/net/phy/smsc.c         |  28 +++---
 drivers/net/usb/asix_devices.c |   6 +-
 drivers/net/usb/smsc95xx.c     | 155 ++++++++++++++++-----------------
 drivers/net/usb/usbnet.c       |   6 +-
 4 files changed, 91 insertions(+), 104 deletions(-)

Comments

Oleksij Rempel April 27, 2022, 12:37 p.m. UTC | #1
On Wed, Apr 27, 2022 at 07:48:00AM +0200, Lukas Wunner wrote:
> Do away with link status polling on LAN95XX USB Ethernet
> and rely on interrupts instead, thereby reducing bus traffic,
> CPU overhead and improving interface bringup latency.
> 
> The meat of the series is in patch [5/7].  The preceding and
> following patches are various cleanups to prepare for and
> adjust to interrupt-driven link state detection.
> 
> Please review and test.  Thanks!

Tested-by: Oleksij Rempel <o.rempel@pengutronix.de>

Tested on:
- LAN9514 (RPi2)
- LAN9512 (EVB)
- LAN9500 (EVB)

On USB unplug i get some not usable kernel message, but it is not the
show stopper for me:
smsc95xx 1-1.4.1:1.0 eth1: Error updating MAC full duplex mode
smsc95xx 1-1.4.1:1.0 eth1: hardware isn't capable of remote wakeup

Thank you!

Regards,
Oleksij
Lukas Wunner April 28, 2022, 12:19 p.m. UTC | #2
On Wed, Apr 27, 2022 at 02:37:15PM +0200, Oleksij Rempel wrote:
> On Wed, Apr 27, 2022 at 07:48:00AM +0200, Lukas Wunner wrote:
> > Do away with link status polling on LAN95XX USB Ethernet
> > and rely on interrupts instead, thereby reducing bus traffic,
> > CPU overhead and improving interface bringup latency.
> > 
> > The meat of the series is in patch [5/7].  The preceding and
> > following patches are various cleanups to prepare for and
> > adjust to interrupt-driven link state detection.
> 
> Tested-by: Oleksij Rempel <o.rempel@pengutronix.de>
> 
> Tested on:
> - LAN9514 (RPi2)
> - LAN9512 (EVB)
> - LAN9500 (EVB)

Thanks a lot for testing, this helps greatly.

> On USB unplug i get some not usable kernel message, but it is not the
> show stopper for me:
> smsc95xx 1-1.4.1:1.0 eth1: Error updating MAC full duplex mode
> smsc95xx 1-1.4.1:1.0 eth1: hardware isn't capable of remote wakeup

Okay, I've amended patch [4/7] to silence the first of these messages
if the error is caused by hot-removal.

The second message isn't related to this series, I'll address that
separately at a later point in time.

Thanks,

Lukas
Ferry Toth May 2, 2022, 8:33 p.m. UTC | #3
Hi,
Op 27-04-2022 om 07:48 schreef Lukas Wunner:
> Do away with link status polling on LAN95XX USB Ethernet
> and rely on interrupts instead, thereby reducing bus traffic,
> CPU overhead and improving interface bringup latency.
> 
> The meat of the series is in patch [5/7].  The preceding and
> following patches are various cleanups to prepare for and
> adjust to interrupt-driven link state detection.
> 
> Please review and test.  Thanks!
> 
> Lukas Wunner (7):
>    usbnet: Run unregister_netdev() before unbind() again
>    usbnet: smsc95xx: Don't clear read-only PHY interrupt
>    usbnet: smsc95xx: Don't reset PHY behind PHY driver's back
>    usbnet: smsc95xx: Avoid link settings race on interrupt reception
>    usbnet: smsc95xx: Forward PHY interrupts to PHY driver to avoid
>      polling
>    net: phy: smsc: Cache interrupt mask
>    net: phy: smsc: Cope with hot-removal in interrupt handler
> 
>   drivers/net/phy/smsc.c         |  28 +++---
>   drivers/net/usb/asix_devices.c |   6 +-
>   drivers/net/usb/smsc95xx.c     | 155 ++++++++++++++++-----------------
>   drivers/net/usb/usbnet.c       |   6 +-
>   4 files changed, 91 insertions(+), 104 deletions(-)
> 

Tested-by: Ferry Toth <fntoth@gmail.com> (Intel Edison-Arduino)

(linux v5.17 + the series + usbnet: smsc95xx: Fix deadlock on runtime 
resume)

While testing I noted another problem. I have "DMA-API: debugging 
enabled by kernel config" and this (I guess) shows me before and after 
the patches:

------------[ cut here ]------------
DMA-API: xhci-hcd xhci-hcd.1.auto: cacheline tracking EEXIST, 
overlapping mappings aren't supported
WARNING: CPU: 0 PID: 24 at kernel/dma/debug.c:570 add_dma_entry+0x1d9/0x270
Modules linked in: mmc_block extcon_intel_mrfld sdhci_pci cqhci sdhci 
led_class mmc_core intel_soc_pmic_mrfld btrfs libcrc32c xor zlib_deflate 
raid6_pq zstd_c>
CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted 5.17.0-edison-acpi-standard #1
Hardware name: Intel Corporation Merrifield/BODEGA BAY, BIOS 542 
2015.01.21:18.19.48
Workqueue: usb_hub_wq hub_event
RIP: 0010:add_dma_entry+0x1d9/0x270
Code: ff 0f 84 97 00 00 00 4c 8b 67 50 4d 85 e4 75 03 4c 8b 27 e8 29 a4 
52 00 48 89 c6 4c 89 e2 48 c7 c7 78 9d 7e 98 e8 b3 f7 b6 00 <0f> 0b 48 
85 ed 0f 85 d3 >
RSP: 0018:ffffb2ce000d7ad0 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 00000000ffffffff RCX: 0000000000000000
RDX: 0000000000000001 RSI: 00000000ffffffea RDI: 00000000ffffffff
RBP: ffff9dea81239a00 R08: ffffffff98b356c8 R09: 00000000ffffdfff
R10: ffffffff98a556e0 R11: ffffffff98a556e0 R12: ffff9dea87397180
R13: 0000000000000001 R14: 0000000000000206 R15: 0000000000046c59
FS:  0000000000000000(0000) GS:ffff9deabe200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00005595807522e0 CR3: 0000000006be2000 CR4: 00000000001006f0
Call Trace:
  <TASK>
  dma_map_page_attrs+0xfc/0x240
  ? usb_hcd_link_urb_to_ep+0x6e/0xa0
  ? preempt_count_add+0x63/0x90
  usb_hcd_map_urb_for_dma+0x3f3/0x580
  usb_hcd_submit_urb+0x93/0xb90
  ? _raw_spin_lock+0xe/0x30
  ? _raw_spin_unlock+0xd/0x20
  ? usb_hcd_link_urb_to_ep+0x6e/0xa0
  ? prepare_transfer+0xff/0x140
  usb_start_wait_urb+0x60/0x160
  usb_control_msg+0xdc/0x140
  hub_ext_port_status+0x82/0x100
  hub_event+0x1b2/0x1880
  ? hub_activate+0x59c/0x880
  process_one_work+0x1d3/0x3a0
  worker_thread+0x48/0x3c0
  ? rescuer_thread+0x380/0x380
  kthread+0xe2/0x110
  ? kthread_complete_and_exit+0x20/0x20
  ret_from_fork+0x22/0x30
  </TASK>
---[ end trace 0000000000000000 ]---
DMA-API: Mapped at:
  debug_dma_map_page+0x60/0xf0
  dma_map_page_attrs+0xfc/0x240
  usb_hcd_map_urb_for_dma+0x3f3/0x580
  usb_hcd_submit_urb+0x93/0xb90
  usb_start_wait_urb+0x60/0x160
usb 1-1.1: new high-speed USB device number 3 using xhci-hcd
usb 1-1.1: New USB device found, idVendor=0424, idProduct=ec00, 
bcdDevice= 2.00
usb 1-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 1-1.1: Product: LAN9514
usb 1-1.1: Manufacturer: SMSC
usb 1-1.1: SerialNumber: 00951d0d

Any ideas on this?
Lukas Wunner May 3, 2022, 8:26 a.m. UTC | #4
On Mon, May 02, 2022 at 10:33:06PM +0200, Ferry Toth wrote:
> Op 27-04-2022 om 07:48 schreef Lukas Wunner:
> > Do away with link status polling on LAN95XX USB Ethernet
> > and rely on interrupts instead, thereby reducing bus traffic,
> > CPU overhead and improving interface bringup latency.
> 
> Tested-by: Ferry Toth <fntoth@gmail.com> (Intel Edison-Arduino)

Thank you!

> While testing I noted another problem. I have "DMA-API: debugging enabled by
> kernel config" and this (I guess) shows me before and after the patches:
> 
> ------------[ cut here ]------------
> DMA-API: xhci-hcd xhci-hcd.1.auto: cacheline tracking EEXIST, overlapping
> mappings aren't supported

That is under investigation here:
https://bugzilla.kernel.org/show_bug.cgi?id=215740

It's apparently a long-standing bug in the USB core which was exposed
by a new WARN() check introduced in 5.14.

Thanks,

Lukas
Ferry Toth May 3, 2022, 2:26 p.m. UTC | #5
Hi

On 03-05-2022 10:26, Lukas Wunner wrote:
> On Mon, May 02, 2022 at 10:33:06PM +0200, Ferry Toth wrote:
>> Op 27-04-2022 om 07:48 schreef Lukas Wunner:
>>> Do away with link status polling on LAN95XX USB Ethernet
>>> and rely on interrupts instead, thereby reducing bus traffic,
>>> CPU overhead and improving interface bringup latency.
>> Tested-by: Ferry Toth <fntoth@gmail.com> (Intel Edison-Arduino)
> Thank you!
>
>> While testing I noted another problem. I have "DMA-API: debugging enabled by
>> kernel config" and this (I guess) shows me before and after the patches:
>>
>> ------------[ cut here ]------------
>> DMA-API: xhci-hcd xhci-hcd.1.auto: cacheline tracking EEXIST, overlapping
>> mappings aren't supported
> That is under investigation here:
> https://bugzilla.kernel.org/show_bug.cgi?id=215740
>
> It's apparently a long-standing bug in the USB core which was exposed
> by a new WARN() check introduced in 5.14.

I'm not sure this is correct. The issue happens for me only when 
connecting the SMSC9414.

Other usb devices I have (memory sticks, wifi, bluetooth) do not trigger 
this.

I think we need to consider it might be a valid warning. It seems to be 
originating from the same "Workqueue: usb_hub_wq hub_event" though.

> Thanks,
>
> Lukas
Lukas Wunner May 4, 2022, 8:15 a.m. UTC | #6
On Tue, May 03, 2022 at 04:26:58PM +0200, Ferry Toth wrote:
> On 03-05-2022 10:26, Lukas Wunner wrote:
> > On Mon, May 02, 2022 at 10:33:06PM +0200, Ferry Toth wrote:
> > > I have "DMA-API: debugging enabled by kernel config"
> > > and this (I guess) shows me before and after the patches:
> > > 
> > > ------------[ cut here ]------------
> > > DMA-API: xhci-hcd xhci-hcd.1.auto: cacheline tracking EEXIST, overlapping
> > > mappings aren't supported
> > 
> > That is under investigation here:
> > https://bugzilla.kernel.org/show_bug.cgi?id=215740
> > 
> > It's apparently a long-standing bug in the USB core which was exposed
> > by a new WARN() check introduced in 5.14.
> 
> I'm not sure this is correct. The issue happens for me only when connecting
> the SMSC9414.
> 
> Other usb devices I have (memory sticks, wifi, bluetooth) do not trigger
> this.

Hm, I've taken the liberty to add that information to the bugzilla.

Thanks,

Lukas