[PATCH] loop.c forgot a kmap

[PATCH] loop.c forgot a kmap

[Benjamin LaHaise]

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:

Re: [PATCH] loop.c forgot a kmap

[William Lee Irwin III]

On Thu, May 23, 2002 at 11:20:25PM -0400, Benjamin LaHaise wrote:
> 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.  

Looks good; IMHO it should be applied.


Cheers,
Bill

Re: [PATCH] loop.c forgot a kmap

[Andrew Morton]

Benjamin LaHaise wrote:
> 
> 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.

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.


-