Messages in this thread | | | Subject | Re: [PATCH 7/8] lguest: trivial guest block driver | From | Rusty Russell <> | Date | Mon, 12 Feb 2007 16:27:14 +1100 |
| |
On Mon, 2007-02-12 at 05:43 +0100, Jens Axboe wrote: > On Mon, Feb 12 2007, Rusty Russell wrote: > > + end_request(bd->req, bd->lb_page->result == 1); > > You are using the old-style end request handling. So while I generally > discourage use of end_request(), you seem to have a bigger problem here:
> > + rq_for_each_bio(bio, req) { > > + struct bio_vec *bvec; > > + bio_for_each_segment(bvec, bio, idx) { > > + BUG_ON(i == LGUEST_MAX_DMA_SECTIONS); > > + BUG_ON(!bvec->bv_len); > > + dma->addr[i] = page_to_phys(bvec->bv_page) > > + + bvec->bv_offset; > > + dma->len[i] = bvec->bv_len; > > + len += bvec->bv_len; > > + i++; > > + } > > + } > > + if (i < LGUEST_MAX_DMA_SECTIONS) > > + dma->len[i] = 0; > > + return len; > > +} > > Here you map the entire request (lets call that segment A..Z), but > end_request() only completes the first chunk of the request. So > elv_next_request() will retrieve the same request again, and you'll then > map B..Z and repeat that transfer. So unless I'm missing some other part > here (just read it over quickly), you are re-doing large parts of a > merged request several times. > > So: don't use end_request(). Add some driver helper that does: > > static void lgb_end_request(struct blockdev *bd) > { > int uptodate = bd->lb_page->result == 1; > struct request *rq = bd->req; > > end_that_request_first(rq, uptodate, req->hard_nr_sectors); > add_disk_randomness(rq->rq_disk); > blkdev_dequeue_request(rq); > end_that_request_last(rq, uptodate); > } > > We could probably even make that a block layer helper, I'm sure others > could be cleaned up with that as well. You want to use that helper in > do_lgb_request() as well.
I'm confused. That code looks like end_request:
void end_request(struct request *req, int uptodate) { if (!end_that_request_first(req, uptodate, req->hard_cur_sectors)) { add_disk_randomness(req->rq_disk); blkdev_dequeue_request(req); end_that_request_last(req, uptodate); } }
Rusty.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |