On Tue, Feb 19 2008, Kyungmin Park wrote:
quoted text
> > Le Tue, 19 Feb 2008 14:48:18 +0900,
> > "Kyungmin Park" <kmpark@infradead.org> a écrit :
> > 
> > > +               /* Write synchronous */
> > > +               bio->bi_rw |= (1 << BIO_RW_SYNC);
> > 
> > Adding BIO_RW_SYNC doesn't make generic_make_request() synchronous as
> > in "generic_make_request() returns only after write completion".
> > BIO_RW_SYNC only asks the I/O layer to unplug immediatly. But
> > generic_make_request() still returns before the completion of the I/O,
> > and the completion is notified asynchronously.
> > 
> 
> Agree, however see the following sequence.
> 
> __generic_make_request call q->make_request_fn(q, bio);
> It was set by blk_init_queue_node with __make_request.
> There are two ways in __make_request.
> Case 1, get_rq
> Case 2, out or merged (otherwise you mean unplug case)
> 
> In case 1, if the BIO_RW_SYNC is set, the request gets the REQ_RW_SYNC
> And REQ_RW_SYNC says 
> "include/linux/blkdev.h":112: __REQ_RW_SYNC,          /* request is sync (O_DIRECT) */
> It means it acts as O_DIRECT flag. Is it right?
> And it also is same as case 2. Unplug the device.
> So next time it hasn't chance to merge???

But that still doesn't make it sync. I think you are working the wrong
way. For ssd we still want merging and plugging also makes sense to some
degree, though it probably should be minimized. It'll only cause an
initial latency, for busy IO workloads you wont be plugging/unplugging
much anyway.

In fact your patch makes things WORSE, since the io schedulers will now
treat the IO as sync and introduce idling for the disk head. And you
definitely don't want that.

-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html