Thread (11 messages) 11 messages, 2 authors, 2018-09-19

Re: [PATCH v7 5/6] block: Make blk_get_request() block for non-PM requests while suspended

From: jianchao.wang <hidden>
Date: 2018-09-19 02:21:17

Hi Bart

On 09/19/2018 01:44 AM, Bart Van Assche wrote:
quoted
quoted
+     * ago. Since blk_queue_enter() is called by the request allocation
+     * code before pm_request_resume(), if no requests have a tag assigned
+     * it is safe to suspend the device.
+     */
+    ret = -EBUSY;
+    if (blk_requests_in_flight(q) == 0) {
+        /*
+         * Call synchronize_rcu() such that later blk_queue_enter()
+         * calls see the pm-only state. See also
+         * https://urldefense.proofpoint.com/v2/url?u=http-3A__lwn.net_Articles_573497_&d=DwIDAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=7WdAxUBeiTUTCy8v-7zXyr4qk7sx26ATvfo6QSTvZyQ&m=o3yai95U5Ge5APoSiMJX64Z8wlI7gf3x0mFnfHpuA4E&s=VPnOWAeo4J4VB944La0uBcynCgVHE-qp52b_9pV8NH4&e=.
+         */
+        synchronize_rcu();
+        if (blk_requests_in_flight(q) == 0)
Seems not safe here.

For blk-mq path:
Someone may have escaped from the preempt checking, missed the blk_pm_request_resume there,
entered into generic_make_request, but have not allocated request and occupied any tag.

There could be a similar scenario for blk-legacy path, the q->nr_pending is increased when
request is queued.

So I guess the q_usage_counter checking is still needed here.
There is only one blk_pm_request_resume() call and that call is inside blk_queue_enter() after the pm_only counter has been checked.

For the legacy block layer, nr_pending is increased after the blk_queue_enter() call from inside blk_old_get_request() succeeded.

So I don't see how blk_pm_request_resume() or q->nr_pending++ could escape from the preempt checking?
For example:


blk_pre_runtime_suspend                    generic_make_request
                                             blk_queue_enter  // success here, no blk_pm_request_resume here
                                             blk_mq_make_requset
blk_set_pm_only

if (blk_requests_in_flight(q) == 0) {
  synchronize_rcu();
  if (blk_requests_in_flight(q) == 0)
    ret = 0;
  }
                                             blk_mq_get_request

Thanks
Jianchao
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help