From patchwork Tue Jun 18 14:53:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oliver Neukum X-Patchwork-Id: 11001967 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 889F7924 for ; Tue, 18 Jun 2019 15:07:24 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 77E5C28AAF for ; Tue, 18 Jun 2019 15:07:24 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6C35B28ABF; Tue, 18 Jun 2019 15:07:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 06FE528AAF for ; Tue, 18 Jun 2019 15:07:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729563AbfFRPHX (ORCPT ); Tue, 18 Jun 2019 11:07:23 -0400 Received: from mx2.suse.de ([195.135.220.15]:44574 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727097AbfFRPHX (ORCPT ); Tue, 18 Jun 2019 11:07:23 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 0FABEAD45; Tue, 18 Jun 2019 15:07:22 +0000 (UTC) Message-ID: <1560869625.3184.11.camel@suse.com> Subject: [RFC] deadlock with flush_work() in UAS From: Oliver Neukum To: Alan Stern Cc: linux-usb@vger.kernel.org Date: Tue, 18 Jun 2019 16:53:45 +0200 X-Mailer: Evolution 3.26.6 Mime-Version: 1.0 Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi, looking at those deadlocks it looks to me like UAS can deadlock on itself. What do you think? Regards Oliver From 2d497f662e6c03fe9e0a75e05b64d52514e890b3 Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Tue, 18 Jun 2019 15:03:56 +0200 Subject: [PATCH] UAS: fix deadlock in error handling and PM flushing work A SCSI error handler and block runtime PM must not allocate memory with GFP_KERNEL. Furthermore they must not wait for tasks allocating memory with GFP_KERNEL. That means that they cannot share a workqueue with arbitrary tasks. Fix this for UAS using a private workqueue. Signed-off0-by: Oliver Neukum --- drivers/usb/storage/uas.c | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 047c5922618f..68b1cb0f84e5 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -80,6 +80,15 @@ static void uas_free_streams(struct uas_dev_info *devinfo); static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *prefix, int status); +/* + * This driver needs its own workqueue, as we need to control memory allocation + * In the course of error handling and power management uas_wait_for_pending_cmnds() + * needs to flush pending work items. In these contexts we cannot allocate + * do block IO and we cannot wait for anybody allocating memory with GFP_KERNEL + * So we have to control all work items that can be on our workqueue we flush + */ +static struct workqueue_struct *workqueue; + static void uas_do_work(struct work_struct *work) { struct uas_dev_info *devinfo = @@ -108,7 +117,7 @@ static void uas_do_work(struct work_struct *work) if (!err) cmdinfo->state &= ~IS_IN_WORK_LIST; else - schedule_work(&devinfo->work); + queue_work(workqueue, &devinfo->work); } out: spin_unlock_irqrestore(&devinfo->lock, flags); @@ -122,7 +131,7 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo) lockdep_assert_held(&devinfo->lock); cmdinfo->state |= IS_IN_WORK_LIST; - schedule_work(&devinfo->work); + queue_work(workqueue, &devinfo->work); } static void uas_zap_pending(struct uas_dev_info *devinfo, int result) @@ -1216,7 +1225,31 @@ static struct usb_driver uas_driver = { .id_table = uas_usb_ids, }; -module_usb_driver(uas_driver); +static int __init uas_init(void) +{ + int rv; + + workqueue = alloc_workqueue("uas", WQ_MEM_RECLAIM, 0); + if (!workqueue) + return -ENOMEM; + + rv = usb_register(&uas_driver); + if (rv) { + destroy_workqueue(workqueue); + return -ENOMEM; + } + + return 0; +} + +static void __exit uas_exit(void) +{ + usb_deregister(&uas_driver); + destroy_workqueue(workqueue); +} + +module_init(uas_init); +module_exit(uas_exit); MODULE_LICENSE("GPL"); MODULE_AUTHOR(