create_bounce() in ll_rw_blk.c

In ll_rw_blk.c's __make_request() there is a call to create_bounce() if CONFIG_HIGHMEM is set. The commentary in that file indicates that this is a temporary fix until 2.5 at which point this would be removed in favour of individual drivers handling this on their own. I've been trying to figure out if a driver I'm working on needs to make this call. That got me wondering...

Is there a reason for pushing this down onto the individual driver writer instead of placing it once and for all in the ll_rw_block() function like:

@@ -1000,6 +1000,10 @@
 	/* Verify requested block sizes. */
 	for (i = 0; i < nr; i++) {
 		struct buffer_head *bh = bhs[i];
+#if CONFIG_HIGHMEM
+		bh = create_bounce(rw, bh);
+		bhs[i] = &bh;
+#endif
 		if (bh->b_size % correct_size) {
 			printk(KERN_NOTICE "ll_rw_block: device %s: "
 			       "only %d-char blocks implemented (%u)\n",

Since the commentary says the driver writer taking HIGHMEM into account could call either create_bounce() or bh_kmap() and the latter deals with bh->b_data, is this something you need to do only if you're accessing bh->b_data? In that case putting the work on the driver writer allows for it to only be done when needed, but are there cases were a buffer_head would pass down out of ll_rw_block() towards a driver that's not ultimately going to read or write the b_data member?

I don't know how all the HIGHMEM/PAE stuff actually works, but I'm guessing that if the heavy handed create_bounce() exists that is because simply doing a bh_kmap() and replacing the bh->b_data at ll_rw_block() time doesn't result in a memory address that would work in the drivers' context? So to get the efficiency of bh_kmap() over create_bounce() you'd have to put the calls in all the drivers?

And since create_bounce() stores the original bh in bh->b_private is this all magically undone then as nested bh->b_end_io's and bh->b_private's unfold themselves with either of bounce_end_io_read() or _write() being called somewhere in there?

Anybody care to comment?

t.

--
*********************************************************

*  tpepper@vato dot org             * Venimus, Vidimus, *
*  http://www.vato.org/~tpepper     * Dolavimus         *

*********************************************************

You _only_ need to make that call if your driver is bypassing the ll_rw_blk supplied make_request_fn. So if you are not using blk_init_queue to specify a request_fn but rather blk_queue_make_request and thus bypassing the I/O scheduler, then you need to make sure that you bounce buffers when you need it.

Yes. b_data is the virtual mapping of b_page (+ offset).

Sure, I can imagine such cases. I could do something like this in my request function:

	unsigned long bus_addr = page_to_bus(bh->b_page) + bh_offset(bh);

which is perfectly good code, and handles highmem which the typical construct

	char *ptr = bh_kmap(bh);
	unsigned long bus_addr = virt_to_bus(ptr);

does not.

The kmap mappings are meant to be short lived. Doing the kmap would work across I/O though, but typically you are unmapping from irq context and then you need to resort to the slower kmap_atomic mappings.

Yep, it's unfolded nicely from b_end_io. bh_bounce->b_end_io will be bounce_end_io_read() for example as you note, which will grab the original bh from bh_bounce->b_private and call the original b_end_io specificed for that buffer.

Jens Axboe