linux-kernel - Re: USB disk speed regression WD Elements - with bisect result 22547c4cc4fe20698a6a85a55b8788859134b8e4

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <Pine.LNX.4.44L0.1707282207480.19177-100000@netrider.rowland.org>
Date:   Fri, 28 Jul 2017 22:09:13 -0400 (EDT)
From:   Alan Stern <stern@...land.harvard.edu>
To:     Zdenek Kabelac <zkabelac@...hat.com>
cc:     USB list <linux-usb@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>, <dianders@...omium.org>,
        <linux@...ck-us.net>, <gregkh@...uxfoundation.org>
Subject: Re: USB disk speed regression WD Elements - with bisect result
 22547c4cc4fe20698a6a85a55b8788859134b8e4

On Fri, 28 Jul 2017, Zdenek Kabelac wrote:

> Dne 28.7.2017 v 20:33 Alan Stern napsal(a):
> > On Thu, 27 Jul 2017, Zdenek Kabelac wrote:
> > 
> >>> Zdenek, you check this explanation by commenting out these last two
> >>> lines at the end of hub_port_connect() in drivers/usb/core/hub.c:
> >>>
> >>> 	if (hcd->driver->relinquish_port && !hub->hdev->parent)
> >>> 		hcd->driver->relinquish_port(hcd, port1);
> >>>
> >>> That should prevent the connection from being handed over to the UHCI
> >>> companion, allowing the device to operate at high speed.
> >>>
> >>
> >> Hi
> >>
> >> Yep - seems this helped - I've dropped revert and commented those 2 lines
> >> and I've used the very same kernel - and speed was all good:
> >>
> >> usb 2-2: new high-speed USB device number 2 using ehci-pci
> >> usb 2-2: new high-speed USB device number 3 using ehci-pci
> >> usb 2-2: New USB device found, idVendor=1058, idProduct=10a8
> >> usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> >>
> >>
> >> So while I'm not sure if this is final fix for USB - this solution surely
> >> solved my WD Element disk attachment issue.
> > 
> > Great!  Here's a real patch to test; I think this will be acceptable if
> > it fixes your problem.
> > 
> > Alan Stern
> > 
> > 
> > 
> > Index: usb-4.x/drivers/usb/core/hub.c
> > ===================================================================
> > --- usb-4.x.orig/drivers/usb/core/hub.c
> > +++ usb-4.x/drivers/usb/core/hub.c
> > @@ -4934,9 +4934,10 @@ loop:
> >   
> >   done:
> >   	hub_port_disable(hub, port1, 1);
> > -	if (hcd->driver->relinquish_port && !hub->hdev->parent)
> > -		hcd->driver->relinquish_port(hcd, port1);
> > -
> > +	if (hcd->driver->relinquish_port && !hub->hdev->parent) {
> > +		if (status != -ENOTCONN && status != -ENODEV)
> > +			hcd->driver->relinquish_port(hcd, port1);
> > +	}
> >   }
> >   
> >   /* Handle physical or logical connection change events.
> > 
> 
> 
> Hi
> 
> Yep - still good speed - however compilation reported this Warning:
> 
> drivers/usb/core/hub.c:4933:37: warning: ‘status’ may be used uninitialized in 
> this function [-Wmaybe-uninitialized]
>     if (status != -ENOTCONN && status != -ENODEV)
> drivers/usb/core/hub.c:4728:6: note: ‘status’ was declared here
>    int status, i;
> 
> 
> So I've just made assignment of 'status = 0' at declaration place -
> but then you have some other unneeded assigns of =0 left around...
> so possibly worth some minor cleanup...

The initialization actually should be -ENODEV.  I have changed the 
patch and will submit it after the weekend.

Alan Stern