From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758029AbcIZIPh (ORCPT ); Mon, 26 Sep 2016 04:15:37 -0400 Received: from mailout2.w1.samsung.com ([210.118.77.12]:61803 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757713AbcIZIPb (ORCPT ); Mon, 26 Sep 2016 04:15:31 -0400 X-AuditID: cbfec7f5-f79ce6d000004c54-35-57e8d91eb23d Subject: Re: [PATCH v3 0/2] Exynos IOMMU: proper runtime PM support (use device dependencies) To: "Rafael J. Wysocki" , Lukas Wunner Cc: Tobias Jakobi , linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, linux-samsung-soc@vger.kernel.org, Joerg Roedel , Inki Dae , Kukjin Kim , Krzysztof Kozlowski , Bartlomiej Zolnierkiewicz , Mark Brown , "Luis R. Rodriguez" , Greg Kroah-Hartman , Tomeu Vizoso , Kevin Hilman From: Marek Szyprowski Message-id: <3c1016f0-fa8d-0874-adfc-4cb9cfad3f02@samsung.com> Date: Mon, 26 Sep 2016 10:15:24 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-version: 1.0 In-reply-to: <1514800.L7P1tHiLfo@vostro.rjw.lan> Content-type: text/plain; charset=utf-8; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA02Se0hTURzHOfe163JxXVa/5mMwKqjIFDJOFlJQdKl/oohFBLb05iOnsqn0 +iNbqSmpKZZYppUp6Hyka4qVptYsp7PUUHso1ZamTMpVZKDlvAr+9/ny/Z7z5Xs4LCkfohVs dFyioIvTxKoYKWW2TNs2+w+NqQNvle7EDwtqaJz/yc5gw/0aBueO5FC4pGUHvnqrVoInxoJw 9pcJEjvqvxC4r+k2g13XniNc0NNM4LFRHzyY40C4y9pL49TK7zTOqupldnnx9tY7BN/4sRTx dRVXGb6lyCjh8wbLET9rkfCmt2kUn2WqQLyrzp8vSDPTB6XHpDsjhNjoZEG3JfSENMpY20ok vFSeKbY6JBfRFGQgDxa4rWC3WgiRV8Hr4RomA0lZOfcAQW5mAyEKF4KnPePE4glrZyESjTIE v02PaVGMIkh9ZUfu1AouDGxl47SbvbkDYOz+R7pDJNdFgctcRLoNhguCDGcG42YZFwr1KU/n KyhuHUxffz/PK7njcOfuICFmvOBP3jDlZg8uEBrTZ+bLSC4Evs5eoUVWQr3ROV8GXD4LI+U/ 5grYOeEHdc9IccIeyBox0CKvgPEOk0RkX+jLy6REzkZw6comkQsQ2JwykXdAe8ebha7lkGu+ SYrXyyA9VS5GePjb/QGJvBuuDxgXHsiKoNpwj8pBysIlcwqXTChcMqEEkRXIW0jSayMFfXCA XqPVJ8VFBoTHa+vQ3Iezznb8akQPLCFtiGORylPGt4+q5bQmWX9W24aAJVXeMkf/mFoui9Cc PSfo4sN0SbGCvg35sJRqtexJSb9azkVqEoXTgpAg6BZdgvVQXETbzufaPQMOFKc015a/MmVn rZ9cdu7Hxy2qI8m+k36oAWLydeebwusvKV+Yw/r2rnnxp2j4UFpTdRWhuCw499mYnNLptStj Zg4/ijJMFSsqf7b73vMYfZc4FdJ40mu7T+aFkG9t+zs7S32OnvocWDQQoJVuKAsuP9bhvJEQ nH/NyO1SUfooTdBGUqfX/AdShtXjbAMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrJIsWRmVeSWpSXmKPExsVy+t/xa7qfbrwINzi8XdFi44z1rBZTHz5h s2hevJ7NYtL9CSwWC/ZbW3TO3sBu8fqFoUX/49fMFk83P2ayuLxrDpvF594jjBYzzu9jsnjx XNrixoSnjBZnTl9itWhb/YHVom/tJTYHQY8nB+cxeey4u4TRY9OqTjaP/XPXsHtMvrGc0ePf MXaPLVfbWTz6tqxi9Pi8Sc5jRvs21gCuKDebjNTElNQihdS85PyUzLx0W6XQEDddCyWFvMTc VFulCF3fkCAlhbLEnFIgz8gADTg4B7gHK+nbJbhlrNlwkKnghHzF/NNP2RsYP0l0MXJySAiY SJw+NYsRwhaTuHBvPVsXIxeHkMASRomtU/uYIJznjBKzj0xi72Lk4BAWiJe4t8UMpEFEwFti zdn/zBA1ZxklJl5eyALiMAucYZF433AGbCybgKFE19suNhCbV8BOYnPjXiYQm0VAVeLnxNtg tqhAjMT+WTOZIWoEJX5MvscCYnMKGEjs6PgLNodZwEziy8vDrBC2vMTmNW+ZJzAKzELSMgtJ 2SwkZQsYmVcxiqSWFuem5xYb6RUn5haX5qXrJefnbmIERvu2Yz+37GDsehd8iFGAg1GJh9fj 8PNwIdbEsuLK3EOMEhzMSiK8T6+8CBfiTUmsrEotyo8vKs1JLT7EaAr0xERmKdHkfGAiyiuJ NzQxNLc0NDK2sDA3MlIS55364Uq4kEB6YklqdmpqQWoRTB8TB6dUA2ODXGzs0sCDpeadEY80 FnFwbmaImvzu8OeXDzv83WTebpTb4bjz9x3TqxJKf74J1v6z8Lpr579Wt/OMxvfpsyJOfJBg rWfaWb7DdQrLzEmOCtX5PkWW81qDT1txBK/5yPT6z+vjHxen+Jj8dDquJlLoriQTLjv7nE5W 5yJ+geZVx+/HaHVHJCmxFGckGmoxFxUnAgD/WaDwDAMAAA== X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20160926081526eucas1p102d216fdee9bd436b61f356c6bc778f5 X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 X-Local-Sender: =?UTF-8?B?TWFyZWsgU3p5cHJvd3NraRtTUlBPTC1LZXJuZWwgKFRQKRs=?= =?UTF-8?B?7IK87ISx7KCE7J6QG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Global-Sender: =?UTF-8?B?TWFyZWsgU3p5cHJvd3NraRtTUlBPTC1LZXJuZWwgKFRQKRtT?= =?UTF-8?B?YW1zdW5nIEVsZWN0cm9uaWNzG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjczOTI=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20160913124917eucas1p23425d2cb1c24e73ae3c2927ec141fd30 X-RootMTR: 20160913124917eucas1p23425d2cb1c24e73ae3c2927ec141fd30 References: <6114649.EDczdxzVVo@vostro.rjw.lan> <20160923135002.GB4077@wunner.de> <1514800.L7P1tHiLfo@vostro.rjw.lan> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Rafael, On 2016-09-24 03:25, Rafael J. Wysocki wrote: > On Friday, September 23, 2016 03:50:02 PM Lukas Wunner wrote: >> On Fri, Sep 23, 2016 at 02:49:20PM +0200, Rafael J. Wysocki wrote: >>> On Tuesday, September 20, 2016 10:51:13 AM Marek Szyprowski wrote: >>>> On 2016-09-19 23:45, Tobias Jakobi wrote: >>>>> I did some tests with the new version today. Sadly the reboot/shutdown >>>>> issues are still present. >>>> Thanks for the report. I've managed to reproduce this issue and it is again >>>> caused by modifying device on devices_kset list before it will be finally >>>> added by device_add(). I thought that the new patchset allows creating >>>> links to a device, which has not been yet added to system device list. >> Hm, Marek, why isn't it possible to set up the links from the consumer's >> ->probe hook in this case? Because consumers are unaware of the IOMMU presence, so they are also unaware of the links that have to be created. >>>> Should it be allowed to create a link to device, which >>>> has not yet been added to system device list by device_add()? >>> While it would be easy to require both the consumer and producer devices to >>> be registered for creating a link between them, that would just make it >>> harder to use links in the first place. >>> >>> So ideally, it should be possible to create links between devices before >>> registering them, but since I didn't take that into account in the current >>> patch series, some quite substantial changes are needed to cover that. >>> >>> Additional link states come to mind, but then the "stateless" links are >>> affected by this problem too. >> device_link_add() could be changed to call device_reorder_to_tail() >> only if device_is_registered(consumer) returns true. >> >> That's an inline function defined in which returns >> dev->kobj.state_in_sysfs, a flag which is set in kobject_add(). > I know what that function is, but using it alone is not sufficient, > because dev->kobj.state_in_sysfs is set before the device is added to > dpm_list. I found that checking for dev->p was enough to check if device has been added to system or not, but this seems to be some kind of ugly workaround: diff --git a/drivers/base/core.c b/drivers/base/core.c index 4542ba9f60d4..780495918b53 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -180,9 +180,11 @@ struct device_link *device_link_add(struct device *consumer, * It is necessary to hold dpm_list locked throughout all that or else * we may end up suspending with a wrong ordering of it. */ - device_pm_lock(); - device_reorder_to_tail(consumer, NULL); - device_pm_unlock(); + if (consumer->p) { + device_pm_lock(); + device_reorder_to_tail(consumer, NULL); + device_pm_unlock(); + } list_add_tail_rcu(&link->s_node, &supplier->links_to_consumers); list_add_tail_rcu(&link->c_node, &consumer->links_to_suppliers); > >> Then device_add() would have to check if any links are already >> set up and reorder the consumer behind the suppliers. >> >> Doesn't seem to be *that* complex, but probably I'm missing something, >> this is just off the cuff... > There are some cases to consider and some races to avoid AFAICS. > > It all gets a lot simpler if device_link_add() is allowed to return NULL when > the supplier device passed to it has not been registered yet. That looks like > a reasonable thing to do to me, but I wonder if someone has a use case in which > it would be a substantial limitation. Hmmm, you are talking here about the supplier, but my case is that supplier is already registered and probed, but the consumer is about to be created. If you think that supporting such case makes no sense, then I will use the workaround with bus notifier I mentioned earlier. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland