Re: [PATCH 2.4] the perils of kunmap_atomic

[Marcelo Tosatti <marcelo.tosatti@cyclades.com>]

On Tue, Oct 26, 2004 at 07:31:24PM -0400, Jeff Garzik wrote:
> 
> kunmap_atomic() violates the Principle of Least Surprise in a nasty way. 
>    kmap(), kunmap(), and kmap_atomic() all take struct page* to 
> reference the memory location.  kunmap_atomic() is the oddball of the 
> three, and takes a kernel address.
> 
> Ignoring the driver-related bugs that are present due to 
> kunmap_atomic()'s weirdness, there also appears to be a big in the 
> !CONFIG_HIGHMEM implementation in 2.4.x.
> 
> (Bart is poking through some of the 2.6.x-related kunmap_atomic slip-ups)
> 
> Anyway, what do people think about the attached patch to 2.4.x?  I'm 
> surprised it has gone unnoticed until now.
> 
> 	Jeff
>
> ===== include/linux/highmem.h 1.12 vs edited =====
> --- 1.12/include/linux/highmem.h	2003-06-30 20:18:42 -04:00
> +++ edited/include/linux/highmem.h	2004-10-26 19:26:14 -04:00
> @@ -70,7 +70,7 @@
>  #define kunmap(page) do { } while (0)
>  
>  #define kmap_atomic(page,idx)		kmap(page)
> -#define kunmap_atomic(page,idx)		kunmap(page)
> +#define kunmap_atomic(addr,idx)		kunmap(virt_to_page(addr))
>  
>  #define bh_kmap(bh)			((bh)->b_data)
>  #define bh_kunmap(bh)			do { } while (0)

Ugh :(

An audit of kunmap_atomic() users is needed.

We can try this in -29pre if there are no objections.

I have no useful comment about the bug itself right now.