From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932429AbcHKBYJ (ORCPT ); Wed, 10 Aug 2016 21:24:09 -0400 Received: from mga09.intel.com ([134.134.136.24]:60671 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751124AbcHKBYH (ORCPT ); Wed, 10 Aug 2016 21:24:07 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,502,1464678000"; d="scan'208";a="1023396635" Subject: Re: [PATCH v2 1/1] usb: misc: usbtest: add fix for driver hang To: Alan Stern References: Cc: Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org From: Lu Baolu Message-ID: <57ABD3B5.8020805@linux.intel.com> Date: Thu, 11 Aug 2016 09:24:05 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 08/10/2016 10:16 PM, Alan Stern wrote: > On Wed, 10 Aug 2016, Lu Baolu wrote: > >> In sg_timeout(), req->status is set to "-ETIMEDOUT" before calling >> into usb_sg_cancel(). usb_sg_cancel() will do nothing and return >> directly if req->status has been set to a non-zero value. This will >> cause driver hang whenever transfer time out is triggered. >> >> This patch fixes this issue. It could be backported to stable kernel >> with version later than v3.15. >> >> Cc: stable@vger.kernel.org # 3.15+ >> Cc: Alan Stern >> Signed-off-by: Lu Baolu >> --- >> v1->v2: >> - Set retval to -ETIMEDOUT after timed out >> - Removed the kernel log in patch description >> >> drivers/usb/misc/usbtest.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c >> index 6b978f0..c273e11 100644 >> --- a/drivers/usb/misc/usbtest.c >> +++ b/drivers/usb/misc/usbtest.c >> @@ -585,7 +585,6 @@ static void sg_timeout(unsigned long _req) >> { >> struct usb_sg_request *req = (struct usb_sg_request *) _req; >> >> - req->status = -ETIMEDOUT; >> usb_sg_cancel(req); >> } >> >> @@ -616,8 +615,10 @@ static int perform_sglist( >> mod_timer(&sg_timer, jiffies + >> msecs_to_jiffies(SIMPLE_IO_TIMEOUT)); >> usb_sg_wait(req); >> - del_timer_sync(&sg_timer); >> - retval = req->status; >> + if (unlikely(!del_timer_sync(&sg_timer))) >> + retval = -ETIMEDOUT; >> + else >> + retval = req->status; >> >> /* FIXME check resulting data pattern */ > I wouldn't bother with the "unlikely()" because this isn't a hot path. > Aside from that, > > Acked-by: Alan Stern Thank you. I will remove "unlikely()" in new version. Best regards, Lu Baolu