From patchwork Thu Feb 29 19:33:49 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Weitao Wang X-Patchwork-Id: 13576981 Received: from mx2.zhaoxin.com (mx2.zhaoxin.com [203.110.167.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3C2DE7B3E4 for ; Thu, 29 Feb 2024 11:33:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=203.110.167.99 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709206437; cv=none; b=RonpA4Xg+jwSx+6VfkurS31Lac/9lyKPbF5zXxGYctmD2OPV7bF77PQjzy3zNErAR6syhcO0E5hFiph1fG0AlLPk1KKoeXMdT/Anah6HCNmjbSSrNQ/xcteTh5UR3EQ5AA1tw3bkeaQkLuZyZvEPfWHhbCoTQElIcQE2Yzwzh1Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709206437; c=relaxed/simple; bh=NJBMQPGpGQBOOoy6SUBMN4VmZHinHKU9LqzKRUr6sJI=; h=From:To:CC:Subject:Date:Message-ID:MIME-Version:Content-Type; b=Xv5qj4l/PW1PrfZ+i0sP8gPyVylB44eGMEpeROTxAoyKHT0lMIF/LancBNGpU7jg3Du/4gOjvcRT6Ofo7HSno/d4qfNwUEiYuJd2a4knOIvT3y2grau+ER6f/g9ray8chwCsrZreXL0+v7TakLJZCHegw9xcPj4mwbab5CQUouQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=zhaoxin.com; spf=pass smtp.mailfrom=zhaoxin.com; arc=none smtp.client-ip=203.110.167.99 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=zhaoxin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=zhaoxin.com X-ASG-Debug-ID: 1709206430-1eb14e0c7d4d010001-YVMibp Received: from ZXSHMBX3.zhaoxin.com (ZXSHMBX3.zhaoxin.com [10.28.252.165]) by mx2.zhaoxin.com with ESMTP id ndFs2NYkvZzLuoYN (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Thu, 29 Feb 2024 19:33:50 +0800 (CST) X-Barracuda-Envelope-From: WeitaoWang-oc@zhaoxin.com X-Barracuda-RBL-Trusted-Forwarder: 10.28.252.165 Received: from zxbjmbx1.zhaoxin.com (10.29.252.163) by ZXSHMBX3.zhaoxin.com (10.28.252.165) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Thu, 29 Feb 2024 19:33:50 +0800 Received: from L440.zhaoxin.com (10.29.8.21) by zxbjmbx1.zhaoxin.com (10.29.252.163) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Thu, 29 Feb 2024 19:33:50 +0800 X-Barracuda-RBL-Trusted-Forwarder: 10.28.252.165 From: Weitao Wang X-Barracuda-RBL-Trusted-Forwarder: 10.29.252.163 To: , , , , , , CC: , Subject: [PATCH v3] USB:UAS:return ENODEV when submit urbs fail with device not attached Date: Fri, 1 Mar 2024 03:33:49 +0800 X-ASG-Orig-Subj: [PATCH v3] USB:UAS:return ENODEV when submit urbs fail with device not attached Message-ID: <20240229193349.5407-1-WeitaoWang-oc@zhaoxin.com> X-Mailer: git-send-email 2.32.0 Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-ClientProxiedBy: zxbjmbx1.zhaoxin.com (10.29.252.163) To zxbjmbx1.zhaoxin.com (10.29.252.163) X-Barracuda-Connect: ZXSHMBX3.zhaoxin.com[10.28.252.165] X-Barracuda-Start-Time: 1709206430 X-Barracuda-Encrypted: ECDHE-RSA-AES128-GCM-SHA256 X-Barracuda-URL: https://10.28.252.36:4443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at zhaoxin.com X-Barracuda-Scan-Msg-Size: 5162 X-Barracuda-BRTS-Status: 1 X-Barracuda-Bayes: INNOCENT GLOBAL 0.0000 1.0000 -2.0210 X-Barracuda-Spam-Score: 1.09 X-Barracuda-Spam-Status: No, SCORE=1.09 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=9.0 tests=DATE_IN_FUTURE_06_12, DATE_IN_FUTURE_06_12_2 X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.121482 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.01 DATE_IN_FUTURE_06_12 Date: is 6 to 12 hours after Received: date 3.10 DATE_IN_FUTURE_06_12_2 DATE_IN_FUTURE_06_12_2 In the scenario of entering hibernation with udisk in the system, if the udisk was gone or resume fail in the thaw phase of hibernation. Its state will be set to NOTATTACHED. At this point, usb_hub_wq was already freezed and can't not handle disconnect event. Next, in the poweroff phase of hibernation, SYNCHRONIZE_CACHE SCSI command will be sent to this udisk when poweroff this scsi device, which will cause uas_submit_urbs to be called to submit URB for sense/data/cmd pipe. However, these URBs will submit fail as device was set to NOTATTACHED state. Then, uas_submit_urbs will return a value SCSI_MLQUEUE_DEVICE_BUSY to the caller. That will lead the SCSI layer go into an ugly loop and system fail to go into hibernation. On the other hand, when we specially check for -ENODEV in function uas_queuecommand_lck, returning DID_ERROR to SCSI layer will cause device poweroff fail and system shutdown instead of entering hibernation. To fix this issue, let uas_submit_urbs to return original generic error when submitting URB failed. At the same time, we need to translate -ENODEV to DID_NOT_CONNECT for the SCSI layer. Suggested-by: Oliver Neukum Cc: stable@vger.kernel.org Signed-off-by: Weitao Wang Acked-by: Oliver Neukum --- v2->v3 - Modify the description of this patch. - An error is returned directly when submitting URB fails. drivers/usb/storage/uas.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 9707f53cfda9..689396777b6f 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -533,7 +533,7 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp, * daft to me. */ -static struct urb *uas_submit_sense_urb(struct scsi_cmnd *cmnd, gfp_t gfp) +static int uas_submit_sense_urb(struct scsi_cmnd *cmnd, gfp_t gfp) { struct uas_dev_info *devinfo = cmnd->device->hostdata; struct urb *urb; @@ -541,16 +541,15 @@ static struct urb *uas_submit_sense_urb(struct scsi_cmnd *cmnd, gfp_t gfp) urb = uas_alloc_sense_urb(devinfo, gfp, cmnd); if (!urb) - return NULL; + return -ENOMEM; usb_anchor_urb(urb, &devinfo->sense_urbs); err = usb_submit_urb(urb, gfp); if (err) { usb_unanchor_urb(urb); uas_log_cmd_state(cmnd, "sense submit err", err); usb_free_urb(urb); - return NULL; } - return urb; + return err; } static int uas_submit_urbs(struct scsi_cmnd *cmnd, @@ -562,9 +561,9 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, lockdep_assert_held(&devinfo->lock); if (cmdinfo->state & SUBMIT_STATUS_URB) { - urb = uas_submit_sense_urb(cmnd, GFP_ATOMIC); - if (!urb) - return SCSI_MLQUEUE_DEVICE_BUSY; + err = uas_submit_sense_urb(cmnd, GFP_ATOMIC); + if (err) + return err; cmdinfo->state &= ~SUBMIT_STATUS_URB; } @@ -572,7 +571,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, cmdinfo->data_in_urb = uas_alloc_data_urb(devinfo, GFP_ATOMIC, cmnd, DMA_FROM_DEVICE); if (!cmdinfo->data_in_urb) - return SCSI_MLQUEUE_DEVICE_BUSY; + return -ENOMEM; cmdinfo->state &= ~ALLOC_DATA_IN_URB; } @@ -582,7 +581,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, if (err) { usb_unanchor_urb(cmdinfo->data_in_urb); uas_log_cmd_state(cmnd, "data in submit err", err); - return SCSI_MLQUEUE_DEVICE_BUSY; + return err; } cmdinfo->state &= ~SUBMIT_DATA_IN_URB; cmdinfo->state |= DATA_IN_URB_INFLIGHT; @@ -592,7 +591,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, cmdinfo->data_out_urb = uas_alloc_data_urb(devinfo, GFP_ATOMIC, cmnd, DMA_TO_DEVICE); if (!cmdinfo->data_out_urb) - return SCSI_MLQUEUE_DEVICE_BUSY; + return -ENOMEM; cmdinfo->state &= ~ALLOC_DATA_OUT_URB; } @@ -602,7 +601,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, if (err) { usb_unanchor_urb(cmdinfo->data_out_urb); uas_log_cmd_state(cmnd, "data out submit err", err); - return SCSI_MLQUEUE_DEVICE_BUSY; + return err; } cmdinfo->state &= ~SUBMIT_DATA_OUT_URB; cmdinfo->state |= DATA_OUT_URB_INFLIGHT; @@ -611,7 +610,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, if (cmdinfo->state & ALLOC_CMD_URB) { cmdinfo->cmd_urb = uas_alloc_cmd_urb(devinfo, GFP_ATOMIC, cmnd); if (!cmdinfo->cmd_urb) - return SCSI_MLQUEUE_DEVICE_BUSY; + return -ENOMEM; cmdinfo->state &= ~ALLOC_CMD_URB; } @@ -621,7 +620,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, if (err) { usb_unanchor_urb(cmdinfo->cmd_urb); uas_log_cmd_state(cmnd, "cmd submit err", err); - return SCSI_MLQUEUE_DEVICE_BUSY; + return err; } cmdinfo->cmd_urb = NULL; cmdinfo->state &= ~SUBMIT_CMD_URB; @@ -698,7 +697,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd) * of queueing, no matter how fatal the error */ if (err == -ENODEV) { - set_host_byte(cmnd, DID_ERROR); + set_host_byte(cmnd, DID_NO_CONNECT); scsi_done(cmnd); goto zombie; }