Re: [PATCH] use clear_page()/copy_page() in favor ofmemset()/memcpy() on whole pages

[Andrew Morton]

On Thu, 02 Sep 2010 13:30:37 +0100
"Jan Beulich" <JBeulich@xxxxxxxxxx> wrote:

> After all that's what they are intended for.
> 
> ...
>
> --- linux-2.6.36-rc3/fs/fuse/dev.c
> +++ 2.6.36-rc3-use-clear_page/fs/fuse/dev.c
> @@ -811,7 +811,7 @@ static int fuse_copy_page(struct fuse_co
>  
>  	if (page && zeroing && count < PAGE_SIZE) {
>  		void *mapaddr = kmap_atomic(page, KM_USER1);
> -		memset(mapaddr, 0, PAGE_SIZE);
> +		clear_page(mapaddr);
>  		kunmap_atomic(mapaddr, KM_USER1);
>  	}
>  	while (count) {

fuse wanted to use clear_highpage() here.  But clear_highpage() uses
KM_USER0.  I don't immediately see why fuse uses KM_USER1 here?

>
> ...
>
> --- linux-2.6.36-rc3/kernel/power/snapshot.c
> +++ 2.6.36-rc3-use-clear_page/kernel/power/snapshot.c
> @@ -988,7 +988,7 @@ static void copy_data_page(unsigned long
>  			 */
>  			safe_copy_page(buffer, s_page);
>  			dst = kmap_atomic(d_page, KM_USER0);
> -			memcpy(dst, buffer, PAGE_SIZE);
> +			copy_page(dst, buffer);
>  			kunmap_atomic(dst, KM_USER0);
>  		} else {
>  			safe_copy_page(page_address(d_page), s_page);
> @@ -1636,7 +1636,7 @@ int snapshot_read_next(struct snapshot_h
>  		memory_bm_position_reset(&orig_bm);
>  		memory_bm_position_reset(&copy_bm);
>  	} else if (handle->cur <= nr_meta_pages) {
> -		memset(buffer, 0, PAGE_SIZE);
> +		clear_page(buffer);
>  		pack_pfns(buffer, &orig_bm);
>  	} else {
>  		struct page *page;
> @@ -1650,7 +1650,7 @@ int snapshot_read_next(struct snapshot_h
>  			void *kaddr;
>  
>  			kaddr = kmap_atomic(page, KM_USER0);
> -			memcpy(buffer, kaddr, PAGE_SIZE);
> +			copy_page(buffer, kaddr);
>  			kunmap_atomic(kaddr, KM_USER0);
>  			handle->buffer = buffer;
>  		} else {
> @@ -1933,7 +1933,7 @@ static void copy_last_highmem_page(void)
>  		void *dst;
>  
>  		dst = kmap_atomic(last_highmem_page, KM_USER0);
> -		memcpy(dst, buffer, PAGE_SIZE);
> +		copy_page(dst, buffer);
>  		kunmap_atomic(dst, KM_USER0);
>  		last_highmem_page = NULL;
>  	}
> @@ -2219,9 +2219,9 @@ swap_two_pages_data(struct page *p1, str
>  
>  	kaddr1 = kmap_atomic(p1, KM_USER0);
>  	kaddr2 = kmap_atomic(p2, KM_USER1);
> -	memcpy(buf, kaddr1, PAGE_SIZE);
> -	memcpy(kaddr1, kaddr2, PAGE_SIZE);
> -	memcpy(kaddr2, buf, PAGE_SIZE);
> +	copy_page(buf, kaddr1);
> +	copy_page(kaddr1, kaddr2);
> +	copy_page(kaddr2, buf);
>  	kunmap_atomic(kaddr1, KM_USER0);
>  	kunmap_atomic(kaddr2, KM_USER1);
>  }

The page-copying functions in snapshot.c are magical.  The changes look
OK to me but I'd ask Rafael to double-check, please.

>
> ...
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/