Re: [PATCH] modular io schedulers with online switching, #2

[Jens Axboe <axboe@suse.de>]

On Sat, Sep 18 2004, Jens Axboe wrote:
> On Fri, Sep 17 2004, Mike Christie wrote:
> > Jens Axboe wrote
> > >+void blk_wait_queue_drained(request_queue_t *q)
> > >+{
> > >+	struct request_list *rl = &q->rq;
> > >+	DEFINE_WAIT(wait);
> > >+
> > >+	set_bit(QUEUE_FLAG_DRAIN, &q->queue_flags);
> > >+
> > >+	prepare_to_wait(&rl->drain, &wait, TASK_UNINTERRUPTIBLE);
> > >+	do {
> > >+		spin_lock_irq(q->queue_lock);
> > >+		if (!rl->count[READ] && !rl->count[WRITE]) {
> > >+			spin_unlock_irq(q->queue_lock);
> > >+			break;
> > >+		}
> > >+
> > >+		__generic_unplug_device(q);
> > >+		spin_unlock_irq(q->queue_lock);
> > >+		io_schedule();
> > >+	} while (1);
> > >+	finish_wait(&rl->drain, &wait);
> > >+}
> > >+
> > 
> > Jens,
> > 
> > If a driver does not allocate requests through blk_get_request, will the 
> > rl->count[] tests need to be changed or do those drivers need to be 
> > changed? For example, if SCSI insterts a special request into the queue, 
> > then someone swaps the io scheduler with no outstanding normal requests 
> > (so the rl->counts will be zero), could the special request still be in 
> > the queue since it allocated its request using kmalloc (the request is 
> > allocated as part of the scsi command).
> 
> Yes, we need to eliminate stack and kmalloc'ed requests for this to be
> completely solid. This is something we have been doing for some time
> already, now would be a good time to fully complete it.
> 
> There's no way to block incoming request at ->add_request_fn() time, so
> we have to do it on merge (first path) or request allocation.

Actually, I already thought of this the other day and there's no problem
even currently with stack or privately allocated requests. These don't
go through blk_alloc_request(), which in turn calls the io scheduler
private ->elv_set_request() function, so they don't have any io
scheduler private memory allocated to them. This also means that they
don't reside inside the io scheduler, but are directly attached to the
queue head.

So I don't think we have a problem there at all, it should just work.

-- 
Jens Axboe