RE: [PATCH] genhd: Do not hold event lock when scheduling workqueue elements

[Dexuan Cui]

> From: linux-block-owner@xxxxxxxxxxxxxxx [mailto:linux-block-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Dexuan Cui
> Sent: Friday, February 3, 2017 20:23
> To: Hannes Reinecke <hare@xxxxxxxx>; Bart Van Assche
> <Bart.VanAssche@xxxxxxxxxxx>; hare@xxxxxxx; axboe@xxxxxxxxx
> Cc: hch@xxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-block@xxxxxxxxxxxxxxx;
> jth@xxxxxxxxxx
> Subject: RE: [PATCH] genhd: Do not hold event lock when scheduling workqueue
> elements
> 
> > From: linux-kernel-owner@xxxxxxxxxxxxxxx [mailto:linux-kernel-
> > owner@xxxxxxxxxxxxxxx] On Behalf Of Hannes Reinecke
> > Sent: Wednesday, February 1, 2017 00:15
> > To: Bart Van Assche <Bart.VanAssche@xxxxxxxxxxx>; hare@xxxxxxx;
> > axboe@xxxxxxxxx
> > Cc: hch@xxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-block@xxxxxxxxxxxxxxx;
> > jth@xxxxxxxxxx
> > Subject: Re: [PATCH] genhd: Do not hold event lock when scheduling
> workqueue
> > elements
> >
> > On 01/31/2017 01:31 AM, Bart Van Assche wrote:
> > > On Wed, 2017-01-18 at 10:48 +0100, Hannes Reinecke wrote:
> > >> @@ -1488,26 +1487,13 @@ static unsigned long
> > disk_events_poll_jiffies(struct gendisk *disk)
> > >>  void disk_block_events(struct gendisk *disk)
> > >>  {
> > >>         struct disk_events *ev = disk->ev;
> > >> -       unsigned long flags;
> > >> -       bool cancel;
> > >>
> > >>         if (!ev)
> > >>                 return;
> > >>
> > >> -       /*
> > >> -        * Outer mutex ensures that the first blocker completes canceling
> > >> -        * the event work before further blockers are allowed to finish.
> > >> -        */
> > >> -       mutex_lock(&ev->block_mutex);
> > >> -
> > >> -       spin_lock_irqsave(&ev->lock, flags);
> > >> -       cancel = !ev->block++;
> > >> -       spin_unlock_irqrestore(&ev->lock, flags);
> > >> -
> > >> -       if (cancel)
> > >> +       if (atomic_inc_return(&ev->block) == 1)
> > >>                 cancel_delayed_work_sync(&disk->ev->dwork);
> > >>
> > >> -       mutex_unlock(&ev->block_mutex);
> > >>  }
> > >
> > > Hello Hannes,
> > >
> > > I have already encountered a few times a deadlock that was caused by the
> > > event checking code so I agree with you that it would be a big step forward
> > > if such deadlocks wouldn't occur anymore. However, this patch realizes a
> > > change that has not been described in the patch description, namely that
> > > disk_block_events() calls are no longer serialized. Are you sure it is safe
> > > to drop the serialization of disk_block_events() calls?
> > >
> > Well, this whole synchronization stuff it a bit weird; I so totally fail
> > to see the rationale for it.
> > But anyway, once we've converted ev->block to atomics I _think_ the
> > mutex_lock can remain; will be checking.
> >
> > Cheers,
> >
> > Hannes
> > --
> 
> Hi, I think I got the same calltrace with today's linux-next (next-20170203).
> 
> The issue happened every time when my Linux virtual machine booted and
> Hannes's patch could NOT help.
> 
> The calltrace is pasted below.
> 
> -- Dexuan
 
Any news on this thread?

The issue is still blocking Linux from booting up normally in my test. :-(

Have we identified the faulty patch?
If so, at least I can try to revert it to boot up.

Thanks,
-- Dexuan