From patchwork Tue May 22 14:12:27 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Liu X-Patchwork-Id: 10418703 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id EBA0660224 for ; Tue, 22 May 2018 14:12:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D7E0228AC4 for ; Tue, 22 May 2018 14:12:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id CC98228B3F; Tue, 22 May 2018 14:12:50 +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=-4.0 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FSL_HELO_FAKE, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=unavailable 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 5200328AC4 for ; Tue, 22 May 2018 14:12:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751624AbeEVOMh (ORCPT ); Tue, 22 May 2018 10:12:37 -0400 Received: from mail-pl0-f65.google.com ([209.85.160.65]:41236 "EHLO mail-pl0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751329AbeEVOMd (ORCPT ); Tue, 22 May 2018 10:12:33 -0400 Received: by mail-pl0-f65.google.com with SMTP id az12-v6so10988089plb.8 for ; Tue, 22 May 2018 07:12:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:subject:message-id:mime-version:content-disposition :user-agent; bh=YBGnD1U2PPx39XUIUmAlK2mcQzEnO4SGrG749weBrzc=; b=Gn8g59jlv55CAPIgtNqC8ZA71RB5zNUluotbZ/lTBm3HXDXt/J1+302GN8DMzypMkT BXX6NXoBKrC/0uFh5JQB3kEHdOIjPbkRvSxIj/rbb9Zr+hKuNwyV2s/V/YMQ0e2/FiuK 2XsW/yvIT1je710yYo19xbDyUp5EgJnbCUKoik8qa2MTYLKFJNSmsQLYweQ7dXp1ZT6L nAK+bLebcjlBfYG4V13KX6JQ3gFONaXf6kUYOGoI6xj/AZBSeqSlIPMpWSLdIemnN5In 2vVUFHVWl66uEEJ7MMa6z15Olbqk9RhIqby65aYzyHnBO8YLbLdbswhGaHY8wMKUG0+N gx9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:subject:message-id:mime-version :content-disposition:user-agent; bh=YBGnD1U2PPx39XUIUmAlK2mcQzEnO4SGrG749weBrzc=; b=b4VqRP6kKuIQrDGMMFIlJJ7P4Hs05i7oZOkEb+M/T7c5T9FGXMuR062Wh+FRxsE+v7 XcC53mJTmNVwhpq8vcYqeW6nXaZzMJ0j9Y+WjXW2WAx6ElS/OWvuJ6Z1g9dQ1Zti1D0J reBy/yCB/ovRVtq+V0JGhMBVa+SsUyAupFLKxNXUIIACo5v2IF8vOCT1nLH5xEzAsIGh oKpQihKlM6MHc3P9s6ZBEt8Enk5e/Vz65NVlytBtgoTjsIgIWyiElaMe9Z7mpX1b0THA hdbp84ufvLVAijjqoHMAQ1j+xH/y2gg/9Vj2iVSfeTC9JaienuT6esDAe5bwGzoMKhp+ 4ouQ== X-Gm-Message-State: ALKqPwewiJczNnEsRj8QgiBzPuUEDlc0bLw8mEQNlSDlcksHq31Bj6OZ 81bCTZzwuKWDoIcUKpbO/z6IvKRz/KA= X-Google-Smtp-Source: AB8JxZoYMx57Gs+df6EnnJj7LYN2vmp7AvVU67Ps/9KF+zNGMCYepWN37q9EPBHHGzuNtkReeqxiUA== X-Received: by 2002:a17:902:82c3:: with SMTP id u3-v6mr24642919plz.83.1526998352436; Tue, 22 May 2018 07:12:32 -0700 (PDT) Received: from google.com ([2401:fa00:fc:202:ac57:d0e0:7094:5]) by smtp.gmail.com with ESMTPSA id g8-v6sm24645520pgc.0.2018.05.22.07.12.30 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 22 May 2018 07:12:31 -0700 (PDT) Date: Tue, 22 May 2018 22:12:27 +0800 From: martin_liu To: gregkh@linuxfoundation.org, stern@rowland.harvard.edu, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [RFC] driver core: don't hold dev's parent lock when using async probe Message-ID: <20180522141227.GA118442@google.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.9.2 (2017-12-15) 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 SOC have internal I/O buses that can't be probed for devices. The devices on the buses can be accessed directly without additinal configuration required. This type of bus is represented as "simple-bus". In some platforms, we name "soc" with "simple-bus" attribute and many devices are hooked under and desribe them in DT (device tree). In commit 'bf74ad5bc417 introduce ("[PATCH] Hold the device's parent's lock during probe and remove")' to solve USB subsystem lock sequence since usb device's characteristic. Thus "soc" needs to be locked whenever a device and driver's probing happen under "soc" bus. During this period, an async driver tries to probe a device which is under the "soc" bus would be blocked until previous driver finish the probing and release "soc" lock. And the next probing under the "soc" bus need to wait for async finish. Because of that, driver's async probe for init time improvement will be shadowed. Since many devices don't have USB devices' characteristic, they actually don't need parent's lock. However, in order to control the risk and minimize the impact, we don't request parent's lock only when a driver requests async probe. Async probe could have more benefit after we have this patch. Signed-off-by: martin_liu --- This RFC is asked to get some feedback since it involed driver core and USB subsystem. I'm not familiar with USB subsystem and not sure if we still need 'bf74ad5bc417 ("[PATCH] Hold the device's parent's lock during probe and remove")' since it has been there over 10 years. If we still need it and hard to fix it , the simple way is to find a place not to allow USB subsystem drivers to have async probe capability. Any suggestion is welcome. drivers/base/bus.c | 19 +++++++++++++------ drivers/base/dd.c | 23 ++++++++++++++++------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/drivers/base/bus.c b/drivers/base/bus.c index ef6183306b40..6434333995d4 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -181,13 +181,15 @@ static ssize_t unbind_store(struct device_driver *drv, const char *buf, struct bus_type *bus = bus_get(drv->bus); struct device *dev; int err = -ENODEV; + bool allow_async; + allow_async = driver_allows_async_probing(drv); dev = bus_find_device_by_name(bus, NULL, buf); if (dev && dev->driver == drv) { - if (dev->parent) /* Needed for USB */ + if (dev->parent && !allow_async)/* Needed for USB */ device_lock(dev->parent); device_release_driver(dev); - if (dev->parent) + if (dev->parent && !allow_async) device_unlock(dev->parent); err = count; } @@ -208,15 +210,17 @@ static ssize_t bind_store(struct device_driver *drv, const char *buf, struct bus_type *bus = bus_get(drv->bus); struct device *dev; int err = -ENODEV; + bool allow_async; + allow_async = driver_allows_async_probing(drv); dev = bus_find_device_by_name(bus, NULL, buf); if (dev && dev->driver == NULL && driver_match_device(drv, dev)) { - if (dev->parent) /* Needed for USB */ + if (dev->parent && !allow_async)/* Needed for USB */ device_lock(dev->parent); device_lock(dev); err = driver_probe_device(drv, dev); device_unlock(dev); - if (dev->parent) + if (dev->parent && !allow_async) device_unlock(dev->parent); if (err > 0) { @@ -769,11 +773,14 @@ EXPORT_SYMBOL_GPL(bus_rescan_devices); */ int device_reprobe(struct device *dev) { + bool allow_async; + if (dev->driver) { - if (dev->parent) /* Needed for USB */ + allow_async = driver_allows_async_probing(dev->driver); + if (dev->parent && !allow_async)/* Needed for USB */ device_lock(dev->parent); device_release_driver(dev); - if (dev->parent) + if (dev->parent && !allow_async) device_unlock(dev->parent); } return bus_rescan_devices_helper(dev, NULL); diff --git a/drivers/base/dd.c b/drivers/base/dd.c index c9f54089429b..36aed1576c58 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -794,6 +794,7 @@ static int __driver_attach(struct device *dev, void *data) { struct device_driver *drv = data; int ret; + bool allow_async; /* * Lock device and try to bind to it. We drop the error @@ -817,13 +818,14 @@ static int __driver_attach(struct device *dev, void *data) return ret; } /* ret > 0 means positive match */ - if (dev->parent) /* Needed for USB */ + allow_async = driver_allows_async_probing(drv); + if (dev->parent && !allow_async)/* Needed for USB */ device_lock(dev->parent); device_lock(dev); if (!dev->driver) driver_probe_device(drv, dev); device_unlock(dev); - if (dev->parent) + if (dev->parent && !allow_async) device_unlock(dev->parent); return 0; @@ -851,19 +853,21 @@ EXPORT_SYMBOL_GPL(driver_attach); static void __device_release_driver(struct device *dev, struct device *parent) { struct device_driver *drv; + bool allow_async; drv = dev->driver; if (drv) { - if (driver_allows_async_probing(drv)) + allow_async = driver_allows_async_probing(drv); + if (allow_async) async_synchronize_full(); while (device_links_busy(dev)) { device_unlock(dev); - if (parent) + if (parent && !allow_async) device_unlock(parent); device_links_unbind_consumers(dev); - if (parent) + if (parent && !allow_async) device_lock(parent); device_lock(dev); @@ -919,7 +923,12 @@ void device_release_driver_internal(struct device *dev, struct device_driver *drv, struct device *parent) { - if (parent) + bool allow_async = false; + + if (drv) + allow_async = driver_allows_async_probing(drv); + + if (parent && !allow_async) device_lock(parent); device_lock(dev); @@ -927,7 +936,7 @@ void device_release_driver_internal(struct device *dev, __device_release_driver(dev, parent); device_unlock(dev); - if (parent) + if (parent && !allow_async) device_unlock(parent); }