Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 2/2] driver core: Fix possible supplier PM-usage counter
 imbalance
To:     "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC:     LKML <linux-kernel@vger.kernel.org>,
        Linux PM <linux-pm@vger.kernel.org>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        Daniel Vetter <daniel@ffwll.ch>,
        Lukas Wunner <lukas@wunner.de>,
        Andrzej Hajda <a.hajda@samsung.com>,
        Russell King - ARM Linux <linux@armlinux.org.uk>,
        Lucas Stach <l.stach@pengutronix.de>,
        Linus Walleij <linus.walleij@linaro.org>,
        Thierry Reding <thierry.reding@gmail.com>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        linux-tegra <linux-tegra@vger.kernel.org>
References: <5510642.nRbR3bcduN@aspire.rjw.lan>
 <9351473.C2nPJoyFsE@aspire.rjw.lan>
From:   Jon Hunter <jonathanh@nvidia.com>
Message-ID: <2ed95b05-317c-59bb-498a-b5481e54bcf6@nvidia.com>
Date:   Fri, 15 Feb 2019 11:00:27 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.4.0
MIME-Version: 1.0
In-Reply-To: <9351473.C2nPJoyFsE@aspire.rjw.lan>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Rafael,

On 12/02/2019 12:08, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> If a stateless device link to a certain supplier with
> DL_FLAG_PM_RUNTIME set in the flags is added and then removed by the
> consumer driver's probe callback, the supplier's PM-runtime usage
> counter will be nonzero after that which effectively causes the
> supplier to remain "always on" going forward.
> 
> Namely, device_link_add() called to add the link invokes
> device_link_rpm_prepare() which notices that the consumer driver is
> probing, so it increments the supplier's PM-runtime usage counter
> with the assumption that the link will stay around until
> pm_runtime_put_suppliers() is called by driver_probe_device(),
> but if the link goes away before that point, the supplier's
> PM-runtime usage counter will remain nonzero.
> 
> To prevent that from happening, first rework pm_runtime_get_suppliers()
> and pm_runtime_put_suppliers() to use the rpm_active refounts of device
> links and make the latter only drop rpm_active and the supplier's
> PM-runtime usage counter for each link by one, unless rpm_active is
> one already for it.  Next, modify device_link_add() to bump up the
> new link's rpm_active refcount and the suppliers PM-runtime usage
> counter by two, to prevent pm_runtime_put_suppliers(), if it is
> called subsequently, from suspending the supplier prematurely (in
> case its PM-runtime usage counter goes down to 0 in there).
> 
> Due to the way rpm_put_suppliers() works, this change does not
> affect runtime suspend of the consumer ends of new device links (or,
> generally, device links for which DL_FLAG_PM_RUNTIME has just been
> set).
> 
> Fixes: e2f3cd831a28 ("driver core: Fix handling of runtime PM flags in device_link_add()")
> Reported-by: Ulf Hansson <ulf.hansson@linaro.org> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> Note that the issue had been there before commit e2f3cd831a28, but it was
> overlooked by that commit and this change is a fix on top of it, so make
> the Fixes: tag point to commit e2f3cd831a28 (instead of an earlier one
> that the patch will not be applicable to).
I noticed that yesterday's and today's -next were no longer booting on
one of our Tegra boards (Tegra210 Jetson TX2) because networking is
failing. The ethernet chip is a USB device and looking at the bootlogs I
can see that the Tegra XHCI driver is failing ...

 tegra-xusb 70090000.usb: xHCI host controller not responding, assume dead
 tegra-xusb 70090000.usb: HC died; cleaning up

The Tegra XHCI driver uses multiple power-domains and uses
device_link_add() to attach them. So now I am wondering if there is
something that we have got wrong in our implementation. However, I don't
see the device being probed deferred on boot or anything like that.

The driver in question is drivers/usb/host/xhci-tegra.c and we add the
links in the function tegra_xusb_powerdomain_init() which is before RPM
is enabled. Let me know if you have any thoughts.

Cheers
Jon

-- 
nvpublic