loop.c forgot a kmap

The patch below fixes a bug in loop.c that causes highmem systems to fail spectacularly when a page happens to be allocated in highmem by replacing the use of page_address with a kmap/kunmap sequence.

-ben

"You will be reincarnated as a toad; and you will be much happier."

:r ~/patches/v2.4.19-pre8-loop-kmap.diff
--- /md0/kernels/2.4/v2.4.19-pre8/drivers/block/loop.c	Wed May  8 13:19:02 2002
+++ v2.4.19-pre8/drivers/block/loop.c	Thu May 23 22:44:09 2002
@@ -199,9 +199,9 @@
 		page = grab_cache_page(mapping, index);
 		if (!page)
 			goto fail;
+		kaddr = kmap(page);
 		if (aops->prepare_write(file, page, offset, offset+size))
 			goto unlock;
-		kaddr = page_address(page);
 		flush_dcache_page(page);
 		transfer_result = lo_do_transfer(lo, WRITE, kaddr + offset, data, size, IV);
 		if (transfer_result) {
@@ -216,6 +216,7 @@
 			goto unlock;
 		if (transfer_result)
 			goto unlock;
+		kunmap(page);
 		data += size;
 		len -= size;
 		offset = 0;
@@ -228,6 +229,7 @@
 	return 0;
 
 unlock:
+	kunmap(page);
 	UnlockPage(page);
 	page_cache_release(page);
 fail:

Looks good; IMHO it should be applied.

Cheers,
Bill

You must be using a funny kernel, or you have loop on a funny address_space.

The current ->prepare_write() API definition requires that the page be kmapped. By ->prepare_write. So it appears that the real bug is actually in whatever you've mounted your loop on.

Now, Linus (I think) decided that this wasn't a good API and moves were made to change it. Note how generic_file_write() kmaps the page as well. So we're currently kmapping pages twice on such rare operations as writing to an ext2 file ;)

So right now, it's unclear whether the kmap should be done by prepare/commit, or whether it should be done by the caller. I started to clean it up. Al disagreed with Linus. I went and did something else.

-