[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <201805220652.BFH82351.SMQFFOJOtFOVLH@I-love.SAKURA.ne.jp>
Date: Tue, 22 May 2018 06:52:21 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Bart.VanAssche@....com, dvyukov@...gle.com
Cc: linux-kernel@...r.kernel.org, linux-block@...r.kernel.org,
jthumshirn@...e.de, alan.christopher.jenkins@...il.com,
syzbot+c4f9cebf9d651f6e54de@...kaller.appspotmail.com,
martin.petersen@...cle.com, axboe@...nel.dk,
dan.j.williams@...el.com, hch@....de, oleksandr@...alenko.name,
ming.lei@...hat.com, martin@...htvoll.de, hare@...e.com,
syzkaller-bugs@...glegroups.com, ross.zwisler@...ux.intel.com,
keith.busch@...el.com, linux-ext4@...r.kernel.org
Subject: Re: INFO: task hung in blk_queue_enter
Bart Van Assche wrote:
> 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@....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.
But waiting there until atomic_read(&q->mq_freeze_depth) becomes 0 is causing deadlock.
wait_event() never returns is a bug. I think we should not wait for q->mq_freeze_depth.
Powered by blists - more mailing lists