On Thu, 2008-08-07 at 20:41 +0200, Jens Axboe wrote:
> I've queued this up for some testing, I'll add the merge support as
> well.
Thanks. Did you pull from my tree? If so I'll provide an incremental
patch. Otherwise I'll go back and recommit it with your suggested
changes.
> > diff --git a/block/blk-barrier.c b/block/blk-barrier.c
> > index a09ead1..29e60ff 100644
> > --- a/block/blk-barrier.c
> > +++ b/block/blk-barrier.c
>
> Not sure why you are placing it here, it should probably just go into
> blk-core.c
Yeah, I vacillated about that for a while -- and even got as far as
moving it there, and back again. It's not really _core_ code either.
Since it started off almost identical to blkdev_issue_flush() I figured
it might as well sit next to it. But I'm happy to move it too.
> > + /* Many callers won't care at all about the outcome. After all,
> > + this is just a hint to the underlying device. They'll just
> > + ignore errors completely. So the end_io function can be just
> > + a call to bio_put() */
> > + if (!end_io)
> > + end_io = (void *)&bio_put;
>
> Please don't do that, that's pretty ugly.
OK.
> > @@ -1488,10 +1502,9 @@ void submit_bio(int rw, struct bio *bio)
> > * If it's a regular read/write or a barrier with data attached,
> > * go through the normal accounting stuff before submission.
> > */
> > - if (!bio_empty_barrier(bio)) {
> > + if (!bio_dataless(bio)) {
> >
> > BIO_BUG_ON(!bio->bi_size);
> > - BIO_BUG_ON(!bio->bi_io_vec);
> >
> > if (rw & WRITE) {
> > count_vm_events(PGPGOUT, count);
>
> Zach suggested bio_has_data() instead, which I agree is a lot more
> readable.
OK.
> > diff --git a/block/elevator.c b/block/elevator.c
> > index ed6f8f3..bb26424 100644
> > --- a/block/elevator.c
> > +++ b/block/elevator.c
> > @@ -675,7 +675,7 @@ void __elv_add_request(struct request_queue *q, struct request *rq, int where,
> > if (q->ordcolor)
> > rq->cmd_flags |= REQ_ORDERED_COLOR;
> >
> > - if (rq->cmd_flags & (REQ_SOFTBARRIER | REQ_HARDBARRIER)) {
> > + if (rq->cmd_flags & (REQ_SOFTBARRIER | REQ_HARDBARRIER | REQ_DISCARD)) {
> > /*
> > * toggle ordered color
> > */
>
> Just make REQ_DISCARD set REQ_SOFTBARRIER? Do you care if this acts as a
> barrier or not?
Er, didn't you object to me setting REQ_SOFTBARRIER for discard
requests, a few lines up? Admittedly, I shouldn't need to do _both_, but
I think I need one or the other.
In the long term no, I don't care if it acts as a barrier. I just did
that until we implement the merge support for discard requests.
> > static inline unsigned int bio_cur_sectors(struct bio *bio)
> > {
> > + if (unlikely(bio_discard(bio)))
> > + return bio->bi_size >> 9;
> > +
> > if (bio->bi_vcnt)
> > return bio_iovec(bio)->bv_len >> 9;
>
> Just make that
>
> if (bio->bi_vcnt)
> return bio_iovec(bio)->bv_len >> 9;
> else
> return bio->bi_size >> 9;
>
> and add a comment about it.
OK.
--
David Woodhouse Open Source Technology Centre
David.Woodhouse@xxxxxxxxx Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
