linux-kernel - Re: [PATCH v5 0/5] Functional dependencies between devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Oct 2016 12:21:53 +0200
From:   Marek Szyprowski <m.szyprowski@...sung.com>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Linux PM list <linux-pm@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alan Stern <stern@...land.harvard.edu>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Tomeu Vizoso <tomeu.vizoso@...labora.com>,
        Mark Brown <broonie@...nel.org>,
        Lukas Wunner <lukas@...ner.de>,
        Kevin Hilman <khilman@...nel.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        "Luis R. Rodriguez" <mcgrof@...e.com>
Subject: Re: [PATCH v5 0/5] Functional dependencies between devices

Hi Rafael,


On 2016-10-19 13:57, Rafael J. Wysocki wrote:
> On Tue, Oct 18, 2016 at 12:46 PM, Marek Szyprowski
> <m.szyprowski@...sung.com> wrote:
>> On 2016-10-10 14:36, Rafael J. Wysocki wrote:
>>> [...]
>>>
>>> One more update after some conversations during LinuxCon Europe.
>>>
>>> The main point was to make it possible for device_link_add() to figure out
>>> the
>>> initial state of the link instead of expecting the caller to provide it
>>> which
>>> might not be reliable enough in general.
>>>
>>> In this version device_link_add() takes three arguments, the supplier and
>>> consumer pointers and flags and it sets the correct initial state of the
>>> link
>>> automatically (unless invoked with the "stateless" flag, of course).  The
>>> cost
>>> is one additional field in struct device (I moved all of the links-related
>>> fields in struct device to a separate sub-structure while at it) to track
>>> the "driver presence status" of the device (to be used by
>>> device_link_add()).
>>>
>>> In addition to that, the links list walks in the core.c and dd.c code are
>>> under the device links mutex now, so the iternal link spinlock is not
>>> needed
>>> any more and I have renamed symbols to distinguish between flags, link
>>> states
>>> and device "driver presence statuses".
>>>
>>> More information is there in the changelogs.
>> Thanks for the update. This version is indeed easier to use and still works
>> fine
>> with my Exynos IOMMU runtime pm rework. You can keep my:
>>
>> Tested-by: Marek Szyprowski <m.szyprowski@...sung.com>
>>
>> I will send updated version of my patchset soon.
> Thanks for the testing, much appreciated!
>
> The series is in a new branch called "device-links-test" in my tree now:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
> device-links-test

While working on integrating IOMMU deferred probing patches I found a bug,
which has been introduced in v4 of device dependency patchset (v3 worked
fine in this area, v5 also contains this bug). The following fixup is
needed to properly create links with DL_FLAG_PM_RUNTIME flag set during
consumer device probing:

From: Marek Szyprowski <m.szyprowski@...sung.com>
Date: Thu, 20 Oct 2016 12:12:14 +0200
Subject: [PATCH] driver core: fix runtime pm state for
  DEVICE_LINK_CONSUMER_PROBE links

If link is added during consumer probe with DL_FLAG_PM_RUNTIME flag set,
the code will do additional pm_runtime_put() on the supplier after
finishing consumer probing. This will break runtime pm operation for
the supplier device. To solve this issue, enforce additional call to
pm_runtime_get_sync() on the supplier device while creating such link.

Signed-off-by: Marek Szyprowski <m.szyprowski@...sung.com>
---
  drivers/base/core.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 48bc5a362f7d..d4cc285a1df7 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -217,6 +217,9 @@ struct device_link *device_link_add(struct device 
*consumer,
          }
      }

+    if (flags & DL_FLAG_PM_RUNTIME && status == DL_STATE_CONSUMER_PROBE)
+        pm_runtime_get_sync(supplier);
+
      /*
       * Move the consumer and all of the devices depending on it to the end
       * of dpm_list and the devices_kset list.
-- 
1.9.1

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland