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
| ||
|
Date: Wed, 8 May 2019 00:49:19 +0900 From: Suwan Kim <suwan.kim027@...il.com> To: shuah <shuah@...nel.org> Cc: valentina.manea.m@...il.com, gregkh@...uxfoundation.org, linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH 1/2] usbip: Remove repeated setting of hcd->state in vhci_bus_suspend() Hi Shuah, On Mon, May 06, 2019 at 09:13:02AM -0600, shuah wrote: > On 5/6/19 6:55 AM, Suwan Kim wrote: > > When hcd suspends execution, hcd_bus_suspend() calls vhci_bus_suspend() > > which sets hcd->state as HC_STATE_SUSPENDED. But after calling > > vhci_bus_suspend(), hcd_bus_suspend() also sets hcd->state as > > HC_STATE_SUSPENDED. > > So, setting hcd->state in vhci_hcd_suspend() is unnecessary. > > > > Signed-off-by: Suwan Kim <suwan.kim027@...il.com> > > --- > > drivers/usb/usbip/vhci_hcd.c | 4 ---- > > 1 file changed, 4 deletions(-) > > > > diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c > > index 667d9c0ec905..e6f378d00fb6 100644 > > --- a/drivers/usb/usbip/vhci_hcd.c > > +++ b/drivers/usb/usbip/vhci_hcd.c > > @@ -1238,10 +1238,6 @@ static int vhci_bus_suspend(struct usb_hcd *hcd) > > dev_dbg(&hcd->self.root_hub->dev, "%s\n", __func__); > > - spin_lock_irqsave(&vhci->lock, flags); > > - hcd->state = HC_STATE_SUSPENDED; > > - spin_unlock_irqrestore(&vhci->lock, flags); > > - > > return 0; > > } > > > > Tell me more about why you think this change is needed? How did you test > this change? I think that host controller specific functions, vhci_bus_resume() or vhci_bus_suspend() in the case of vhci, usually process host controller specific data (struct vhci_hcd) not a generic data (struct usb_hcd). The generic data is usually processed by generic HCD layer. But vhci_bus_resume() and vhci_bus_suspend() set generic data (hcd->state) and moreover this variable is set in generic HCD layer once again(hcd_bus_resume() and hcd_bus_suspend()). So, i think host controller specific functions (vhci_bus_resume() and vhci_bus_suspend()) don't need to set the generic data that is "hcd->state = HC_STATE_RUNNING or HC_STATE_SUSPEND". For test, I loaded vhci-hcd module, suspended and resumed my computer and checked hcd->state variable. There is no difference compared with not modified version because my patch just removes repeated and unnecessary part. Regards Suwan Kim
Powered by blists - more mailing lists