lkml.org 
[lkml]   [2018]   [May]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: INFO: task hung in blk_queue_enter
    Date
    On Wed, 2018-05-16 at 17:16 +0200, Dmitry Vyukov wrote:
    > On Wed, May 16, 2018 at 4:56 PM, Bart Van Assche <Bart.VanAssche@wdc.com> wrote:
    > > On Wed, 2018-05-16 at 22:05 +0900, Tetsuo Handa wrote:
    > > > diff --git a/block/blk-core.c b/block/blk-core.c
    > > > index 85909b4..59e2496 100644
    > > > --- a/block/blk-core.c
    > > > +++ b/block/blk-core.c
    > > > @@ -951,10 +951,10 @@ int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags)
    > > > smp_rmb();
    > > >
    > > > wait_event(q->mq_freeze_wq,
    > > > - (atomic_read(&q->mq_freeze_depth) == 0 &&
    > > > - (preempt || !blk_queue_preempt_only(q))) ||
    > > > + atomic_read(&q->mq_freeze_depth) ||
    > > > + (preempt || !blk_queue_preempt_only(q)) ||
    > > > blk_queue_dying(q));
    > > > - if (blk_queue_dying(q))
    > > > + if (atomic_read(&q->mq_freeze_depth) || blk_queue_dying(q))
    > > > return -ENODEV;
    > > > }
    > > > }
    > >
    > > That change looks wrong to me.
    >
    > Hi Bart,
    >
    > Why does it look wrong to you?

    Because that change conflicts with the purpose of queue freezing and also because
    that change would inject I/O errors in code paths that shouldn't inject I/O errors.
    Please have a look at e.g. generic_make_request(). From the start of that function:

    if (blk_queue_enter(q, flags) < 0) {
    if (!blk_queue_dying(q) && (bio->bi_opf & REQ_NOWAIT))
    bio_wouldblock_error(bio);
    else
    bio_io_error(bio);
    return ret;
    }

    The above patch changes the behavior of blk_queue_enter() code from waiting while
    q->mq_freeze_depth != 0 into returning -ENODEV while the request queue is frozen.
    That will cause generic_make_request() to call bio_io_error(bio) while a request
    queue is frozen if REQ_NOWAIT has not been set, which is the default behavior. So
    any operation that freezes the queue temporarily, e.g. changing the queue depth,
    concurrently with I/O processing can cause I/O to fail with -ENODEV. As you
    probably know failure of write requests has very annoying consequences. It e.g.
    causes filesystems to go into read-only mode. That's why I think that the above
    change is completely wrong.

    Bart.


    \
     
     \ /
      Last update: 2018-05-16 17:38    [W:5.142 / U:0.300 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site