From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Alan Stern <stern@rowland.harvard.edu>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Pavel Machek <pavel@ucw.cz>, Len Brown <len.brown@intel.com>,
linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
Thierry Reding <thierry.reding@gmail.com>
Subject: Re: [PATCH 1/2] PM / sleep: ensure deferred probe workqueue is finished in wait_for_device_probe
Date: Fri, 9 Oct 2015 09:38:13 -0500 [thread overview]
Message-ID: <5617D155.60700@ti.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1510081637570.1284-100000@iolanthe.rowland.org>
On 10/08/2015 03:53 PM, Alan Stern wrote:
> On Thu, 8 Oct 2015, Rafael J. Wysocki wrote:
>
>>> @@ -391,6 +391,10 @@ int driver_probe_done(void)
>>> */
>>> void wait_for_device_probe(void)
>>> {
>>> + /* wait for the deferred probe workqueue to finish */
>>> + if (driver_deferred_probe_enable)
>>> + flush_workqueue(deferred_wq);
>>> +
>>> /* wait for the known devices to complete their probing */
>>> wait_event(probe_waitqueue, atomic_read(&probe_count) == 0);
>>> async_synchronize_full();
>>
>> I'm not sure if this is sufficient.
>>
>> Something may be added to the workqueue right after you've flushed it and
>> then be reporobed after the wait_event() in theory. Or am I missing anything?
>
> Maybe I'm missing part of this, but I think the point is to make sure
> that every probe which began or was queued before this function got
> called, has finished before the function returns.
>
> Thus, in the case at hand we want to defer all probes starting from
> some point in the system sleep transition. Grygorii sets his
> defer_all_probes variable and then calls this function. It waits for
> any probes that were initiated before the function call. Any probe
> that was initiated after the function call (for example, the ones
> you're concerned about between the flush_workqueue and wait_event) will
> see that defer_all_probes is set and so will defer itself.
Yes. It will work as expected with the next patch.
For all other case, where this API is used alone -
it will make things more safe, but there is no way to completely block
scheduling of new probes.
>
> Now, I'm not sure what happens when a probe that was deferred tries to
> defer itself again. Do we end up in an infinite probing loop?
No. handling of defered probes will be re triggered only if
some probe was finished successfully.
> Is the deferred_wq workqueue freezable?
seems WQ_FREEZABLE is not set for this WQ.
--
regards,
-grygorii
WARNING: multiple messages have this Message-ID (diff)
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Alan Stern <stern@rowland.harvard.edu>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Pavel Machek <pavel@ucw.cz>, Len Brown <len.brown@intel.com>,
<linux-kernel@vger.kernel.org>, <linux-pm@vger.kernel.org>,
Thierry Reding <thierry.reding@gmail.com>
Subject: Re: [PATCH 1/2] PM / sleep: ensure deferred probe workqueue is finished in wait_for_device_probe
Date: Fri, 9 Oct 2015 09:38:13 -0500 [thread overview]
Message-ID: <5617D155.60700@ti.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1510081637570.1284-100000@iolanthe.rowland.org>
On 10/08/2015 03:53 PM, Alan Stern wrote:
> On Thu, 8 Oct 2015, Rafael J. Wysocki wrote:
>
>>> @@ -391,6 +391,10 @@ int driver_probe_done(void)
>>> */
>>> void wait_for_device_probe(void)
>>> {
>>> + /* wait for the deferred probe workqueue to finish */
>>> + if (driver_deferred_probe_enable)
>>> + flush_workqueue(deferred_wq);
>>> +
>>> /* wait for the known devices to complete their probing */
>>> wait_event(probe_waitqueue, atomic_read(&probe_count) == 0);
>>> async_synchronize_full();
>>
>> I'm not sure if this is sufficient.
>>
>> Something may be added to the workqueue right after you've flushed it and
>> then be reporobed after the wait_event() in theory. Or am I missing anything?
>
> Maybe I'm missing part of this, but I think the point is to make sure
> that every probe which began or was queued before this function got
> called, has finished before the function returns.
>
> Thus, in the case at hand we want to defer all probes starting from
> some point in the system sleep transition. Grygorii sets his
> defer_all_probes variable and then calls this function. It waits for
> any probes that were initiated before the function call. Any probe
> that was initiated after the function call (for example, the ones
> you're concerned about between the flush_workqueue and wait_event) will
> see that defer_all_probes is set and so will defer itself.
Yes. It will work as expected with the next patch.
For all other case, where this API is used alone -
it will make things more safe, but there is no way to completely block
scheduling of new probes.
>
> Now, I'm not sure what happens when a probe that was deferred tries to
> defer itself again. Do we end up in an infinite probing loop?
No. handling of defered probes will be re triggered only if
some probe was finished successfully.
> Is the deferred_wq workqueue freezable?
seems WQ_FREEZABLE is not set for this WQ.
--
regards,
-grygorii
next prev parent reply other threads:[~2015-10-09 14:38 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-08 16:57 [PATCH 0/2] PM / sleep: prohibit devices probing during suspend/hibernation Grygorii Strashko
2015-10-08 16:57 ` Grygorii Strashko
2015-10-08 16:57 ` [PATCH 1/2] PM / sleep: ensure deferred probe workqueue is finished in wait_for_device_probe Grygorii Strashko
2015-10-08 16:57 ` Grygorii Strashko
2015-10-08 20:50 ` Rafael J. Wysocki
2015-10-08 20:53 ` Alan Stern
2015-10-08 20:53 ` Alan Stern
2015-10-09 14:38 ` Grygorii Strashko [this message]
2015-10-09 14:38 ` Grygorii Strashko
2015-10-09 21:16 ` Rafael J. Wysocki
2015-10-13 18:25 ` Grygorii Strashko
2015-10-13 18:25 ` Grygorii Strashko
2015-10-08 16:57 ` [PATCH 2/2] PM / sleep: prohibit devices probing during suspend/hibernation Grygorii Strashko
2015-10-08 16:57 ` Grygorii Strashko
2015-10-08 17:24 ` Alan Stern
2015-10-08 17:24 ` Alan Stern
2015-10-08 18:54 ` Grygorii Strashko
2015-10-08 18:54 ` Grygorii Strashko
2015-10-08 19:20 ` Alan Stern
2015-10-08 19:20 ` Alan Stern
2015-10-08 19:48 ` Grygorii Strashko
2015-10-08 19:48 ` Grygorii Strashko
2015-10-08 20:05 ` Alan Stern
2015-10-08 20:05 ` Alan Stern
2015-10-08 20:46 ` Rafael J. Wysocki
2015-10-09 14:31 ` Grygorii Strashko
2015-10-09 14:31 ` Grygorii Strashko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5617D155.60700@ti.com \
--to=grygorii.strashko@ti.com \
--cc=gregkh@linuxfoundation.org \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=pavel@ucw.cz \
--cc=rjw@rjwysocki.net \
--cc=stern@rowland.harvard.edu \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.