All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>,
	Shirish Pargaonkar <shirishpargaonkar@gmail.com>,
	Jens Axboe <axboe@kernel.dk>,
	SCSI development list <linux-scsi@vger.kernel.org>,
	Kernel development list <linux-kernel@vger.kernel.org>
Subject: Re: WARNING in block layer triggered in 3.17-rc3
Date: Tue, 9 Sep 2014 10:19:45 +0900	[thread overview]
Message-ID: <20140909011945.GD11706@mtj.dyndns.org> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1409081335450.1433-100000@iolanthe.rowland.org>

Hello,

On Mon, Sep 08, 2014 at 01:42:44PM -0400, Alan Stern wrote:
> > This looks like a nasty hack.  In theory the QUEUE_FLAG_INIT_DONE should
> > be unset on blk_unregister_queue() to match the teardown; it's only
> > accident it isn't.  del_gendisk() in sd_remove() is supposed to tear a
> > lot of queue stuff down.
> 
> It's not clear what the operative assumptions are.  The comment in
> blk_register_queue() implies that bypass is active only because it was
> set up that way when the queue was created.  The fact that
> blk_unregister_queue() doesn't call blk_queue_bypass_start() seems to
> support this view -- although it could also be a simple oversight.
> 
> Hopefully Tejun can clear this iup.

Maintaining the initial bypass till queue registration is an
optimization because shutting down a fully functional queue is a
costly operation and there are drivers which set and destroy queues
repeatedly while probing, so, yeah, it's really a special case for
when the queue is being registered for the first time.

> > Your hack seems to indicate that this doesn't work on the add->del->add
> > transtion of a gendisk.
> 
> Indeed, it does not work.

As such, the change you suggested makes perfect sense to me.  Why
wouldn't it work?

Thanks.

-- 
tejun

  reply	other threads:[~2014-09-09  1:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-05 17:45 WARNING in block layer triggered in 3.17-rc3 Alan Stern
2014-09-07 10:43 ` Ming Lei
2014-09-07 22:59 ` Shirish Pargaonkar
2014-09-08 14:51   ` Alan Stern
2014-09-08 15:05     ` Shirish Pargaonkar
2014-09-08 15:51     ` James Bottomley
2014-09-08 16:11       ` Shirish Pargaonkar
2014-09-08 17:42       ` Alan Stern
2014-09-09  1:19         ` Tejun Heo [this message]
2014-09-09 15:08           ` Alan Stern
2014-09-09 15:17             ` Tejun Heo
2014-09-09 15:50               ` [PATCH] Block: fix unbalanced bypass-disable in blk_register_queue Alan Stern
2014-09-09 16:43                 ` Tejun Heo
2014-09-09 18:32                 ` Shirish Pargaonkar

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=20140909011945.GD11706@mtj.dyndns.org \
    --to=tj@kernel.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=axboe@kernel.dk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=shirishpargaonkar@gmail.com \
    --cc=stern@rowland.harvard.edu \
    /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.