PATCH - change to blkdev->queue calling triggers BUG in md.c

2 messages, 2 authors, 2002-09-02 · open the first message on its own page

PATCH - change to blkdev->queue calling triggers BUG in md.c

From: Neil Brown <hidden>
Date: 2002-09-01 23:39:22

Changeset 1.573 (just prior to 2.5.33 release) changed the calling
sequence for blk_dev[major].queue so that it is now called before the 
bd_op->open function is called.
This triggers a BUG in md.c which checked that the device was open
whenever ->queue was called.  Patch below removes the BUG.

I'm actually a little disappointed by this change.  I was hoping that
the ->queue might get changed to be passed a 'struct block_device *'
instead of a 'kdev_t' so that the device driver would only have to
interpret the device number in one place: the open.  But now that
->queue is called before ->open, that wouldn't help.

I don't suppose it would make sense to do the default:
	if (!bdev->bd_queue) {
		struct blk_dev_struct *p = blk_dev + major(dev);
		bdev->bd_queue = &p->request_queue;
	}
bit where it is now, and leave the:
		if (p->queue)
			bdev->bd_queue =  p->queue(dev);

bit until after the open?  It would keep floppy happy, and make me
happy too, but I'm not sure that it is actually 'right'...

Anyway, here is the patch that stops md from BUGging out.

NeilBrown

### Comments for ChangeSet
Remove BUG in md.c that change in 2.5.33 triggers.

Since 2.5.33, the blk_dev[].queue is called without
the device open, so md_queue_proc can no-longer assume
that the device is open.


 ----------- Diffstat output ------------
 ./drivers/md/md.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
--- ./drivers/md/md.c	2002/09/01 23:27:10	1.1
+++ ./drivers/md/md.c	2002/09/01 23:28:27	1.2
@@ -3157,11 +3157,11 @@ request_queue_t * md_queue_proc(kdev_t d
 {
 	mddev_t *mddev = mddev_find(minor(dev));
 	request_queue_t *q = BLK_DEFAULT_QUEUE(MAJOR_NR);
-	if (!mddev || atomic_read(&mddev->active)<2)
-		BUG();
-	if (mddev->pers)
-		q = &mddev->queue;
-	mddev_put(mddev); /* the caller must hold a reference... */
+	if (mddev) {
+		if (mddev->pers)
+			q = &mddev->queue;
+		mddev_put(mddev);
+	}
 	return q;
 }
 

Re: PATCH - change to blkdev->queue calling triggers BUG in md.c

From: Linus Torvalds <torvalds@transmeta.com>
Date: 2002-09-02 04:00:54

On Mon, 2 Sep 2002, Neil Brown wrote:
I'm actually a little disappointed by this change.  I was hoping that
the ->queue might get changed to be passed a 'struct block_device *'
instead of a 'kdev_t' so that the device driver would only have to
interpret the device number in one place: the open.  But now that
->queue is called before ->open, that wouldn't help.
We may still do this. 

Right now the _only_ reason to call ->queue before open() is that open() 
is also doing things like disk change checking, which reasonably needs the 
queue because it can need to do IO in order to check the disk change 
status. The floppy in fact did exactly this.

HOWEVER, that disk change checking really should be done by the generic
layers, and it should be done after the open() anyway (and not by the
open), and I think Al is actually working on this. That will allow us to 
be a bit more flexible about the ordering.

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