[PATCH] bio kmapping changes

RAID5 is calling copy_data() under sh->lock.  But copy_data() does kmap(),
which can sleep.

The best fix is to use kmap_atomic() in there.  It is faster than kmap() and
does not block.

The patch removes the unused bio_kmap() and replaces __bio_kmap() with
__bio_kmap_atomic().  I think it's best to withdraw the sleeping-and-slow
bio_kmap() from the kernel API before someone else tries to use it.


Also, I notice that bio_kmap_irq() was using local_save_flags().  This is a
bug - local_save_flags() does not disable interrupts.  Converted that to
local_irq_save().  These names are terribly chosen.

This patch was acked by Jens and Neil.

Documentation/block/biodoc.txt

@@ -204,7 +204,7 @@ may need to abort DMA operations and revert to PIO for the transfer, in
 which case a virtual mapping of the page is required. For SCSI it is also
 done in some scenarios where the low level driver cannot be trusted to
 handle a single sg entry correctly. The driver is expected to perform the
-kmaps as needed on such occasions using the bio_kmap and bio_kmap_irq
+kmaps as needed on such occasions using the __bio_kmap_atomic and bio_kmap_irq
 routines as appropriate. A driver could also use the blk_queue_bounce()
 routine on its own to bounce highmem i/o to low memory for specific requests
 if so desired.
@@ -1147,9 +1147,9 @@ use blk_rq_map_sg for scatter gather) to be able to ship it to the driver. For
 PIO drivers (or drivers that need to revert to PIO transfer once in a
 while (IDE for example)), where the CPU is doing the actual data
 transfer a virtual mapping is needed. If the driver supports highmem I/O,
-(Sec 1.1, (ii) ) it needs to use bio_kmap and bio_kmap_irq to temporarily
-map a bio into the virtual address space. See how IDE handles this with
-ide_map_buffer.
+(Sec 1.1, (ii) ) it needs to use __bio_kmap_atomic and bio_kmap_irq to
+temporarily map a bio into the virtual address space. See how IDE handles
+this with ide_map_buffer.
 
 
 8. Prior/Related/Impacted patches

drivers/block/ll_rw_blk.c

@@ -188,7 +188,7 @@ void blk_queue_merge_bvec(request_queue_t *q, merge_bvec_fn *mbfn)
  * Caveat:
  *    The driver that does this *must* be able to deal appropriately
  *    with buffers in "highmemory". This can be accomplished by either calling
- *    bio_kmap() to get a temporary kernel mapping, or by calling
+ *    __bio_kmap_atomic() to get a temporary kernel mapping, or by calling
  *    blk_queue_bounce() to create a buffer in normal memory.
  **/
 void blk_queue_make_request(request_queue_t * q, make_request_fn * mfn)

drivers/md/raid5.c

@@ -21,6 +21,7 @@
 #include <linux/slab.h>
 #include <linux/raid/raid5.h>
 #include <linux/bio.h>
+#include <linux/highmem.h>
 #include <asm/bitops.h>
 #include <asm/atomic.h>
 
@@ -634,12 +635,12 @@ static void copy_data(int frombio, struct bio *bio,
 			else clen = len;
 			
 			if (clen > 0) {
-				char *ba = __bio_kmap(bio, i);
+				char *ba = __bio_kmap_atomic(bio, i, KM_USER0);
 				if (frombio)
 					memcpy(pa+page_offset, ba+b_offset, clen);
 				else
 					memcpy(ba+b_offset, pa+page_offset, clen);
-				__bio_kunmap(bio, i);
+				__bio_kunmap_atomic(ba, KM_USER0);
 			}	
 			if (clen < len) /* hit end of page */
 				break;

include/linux/bio.h

@@ -148,10 +148,11 @@ struct bio {
  * permanent PIO fall back, user is probably better off disabling highmem
  * I/O completely on that queue (see ide-dma for example)
  */
-#define __bio_kmap(bio, idx) (kmap(bio_iovec_idx((bio), (idx))->bv_page) + bio_iovec_idx((bio), (idx))->bv_offset)
-#define bio_kmap(bio)	__bio_kmap((bio), (bio)->bi_idx)
-#define __bio_kunmap(bio, idx)	kunmap(bio_iovec_idx((bio), (idx))->bv_page)
-#define bio_kunmap(bio)		__bio_kunmap((bio), (bio)->bi_idx)
+#define __bio_kmap_atomic(bio, idx, kmtype)				\
+	(kmap_atomic(bio_iovec_idx((bio), (idx))->bv_page, kmtype) +	\
+		bio_iovec_idx((bio), (idx))->bv_offset)
+
+#define __bio_kunmap_atomic(addr, kmtype) kunmap_atomic(addr, kmtype)
 
 /*
  * merge helpers etc
@@ -238,7 +239,7 @@ extern inline char *bio_kmap_irq(struct bio *bio, unsigned long *flags)
 	 * might not be a highmem page, but the preempt/irq count
 	 * balancing is a lot nicer this way
 	 */
-	local_save_flags(*flags);
+	local_irq_save(*flags);
 	addr = (unsigned long) kmap_atomic(bio_page(bio), KM_BIO_SRC_IRQ);
 
 	if (addr & ~PAGE_MASK)