On Mon, 30 Apr 2007, Satyam Sharma wrote:
> A simple "Replace open-coded kmap_atomic() and kunmap_atomic()
> surrounding two memory clear operations with zero_user_page(), as both
> memory operations act on the same page" would have been better.
Ok.
> Perhaps you were more worried with the additional overhead of two
> successive kmap_atomic() and kunmap_atomic() calls for the same page
> in the two resulting zero_user_page()'s (if both conditions evaluate
> to true for the same page), but that would still be a price to pay to
> replace the current open-coding.
Right.
> Either you've fixed an issue (in which case this should've been a
> different patch with the accompanying explanation) or else I don't see
> what we gain with the additional if's. Again, we still do incur the
> overhead of two successive kmap_atomic() / kunmap_atomic() calls for
> the same page in order to replace the open-coding.
We also flush the page twice. I think its worth the savings.
Revised patch (fixes missing { due to missing quilt refresh):
zero_user_page: Use it in more places
Replace open-coded kmap_atomic() and kunmap_atomic()
surrounding two memory clear operations with zero_user_page(), as both
memory operations act on the same page.
Cc: Nate Diller <[EMAIL PROTECTED]>
Signed-off-by: Christoph Lameter <[EMAIL PROTECTED]>
---
fs/buffer.c | 29 ++++++++++-------------------
fs/libfs.c | 10 +++++-----
2 files changed, 15 insertions(+), 24 deletions(-)
Index: linux-2.6.21-rc7-mm2/fs/buffer.c
===================================================================
--- linux-2.6.21-rc7-mm2.orig/fs/buffer.c 2007-04-27 22:51:27.000000000
-0700
+++ linux-2.6.21-rc7-mm2/fs/buffer.c 2007-04-29 23:58:48.000000000 -0700
@@ -1796,19 +1796,12 @@ static int __block_prepare_write(struct
set_buffer_uptodate(bh);
continue;
}
- if (block_end > to || block_start < from) {
- void *kaddr;
-
- kaddr = kmap_atomic(page, KM_USER0);
- if (block_end > to)
- memset(kaddr+to, 0,
- block_end-to);
- if (block_start < from)
- memset(kaddr+block_start,
- 0, from-block_start);
- flush_dcache_page(page);
- kunmap_atomic(kaddr, KM_USER0);
- }
+ if (block_end > to)
+ zero_user_page(page, to,
+ block_end - to, KM_USER0);
+ if (block_start < from)
+ zero_user_page(page, block_start,
+ from - block_start, KM_USER0);
continue;
}
}
@@ -2224,7 +2217,6 @@ int nobh_prepare_write(struct page *page
unsigned block_in_page;
unsigned block_start;
sector_t block_in_file;
- char *kaddr;
int nr_reads = 0;
int i;
int ret = 0;
@@ -2264,13 +2256,12 @@ int nobh_prepare_write(struct page *page
if (PageUptodate(page))
continue;
if (buffer_new(&map_bh) || !buffer_mapped(&map_bh)) {
- kaddr = kmap_atomic(page, KM_USER0);
if (block_start < from)
- memset(kaddr+block_start, 0, from-block_start);
+ zero_user_page(page, block_start,
+ from - block_start, KM_USER0);
if (block_end > to)
- memset(kaddr + to, 0, block_end - to);
- flush_dcache_page(page);
- kunmap_atomic(kaddr, KM_USER0);
+ zero_user_page(page, to,
+ block_end - to, KM_USER0);
continue;
}
if (buffer_uptodate(&map_bh))
Index: linux-2.6.21-rc7-mm2/fs/libfs.c
===================================================================
--- linux-2.6.21-rc7-mm2.orig/fs/libfs.c 2007-04-27 22:51:27.000000000
-0700
+++ linux-2.6.21-rc7-mm2/fs/libfs.c 2007-04-27 22:55:12.000000000 -0700
@@ -338,11 +338,11 @@ int simple_prepare_write(struct file *fi
{
if (!PageUptodate(page)) {
if (to - from != PAGE_CACHE_SIZE) {
- void *kaddr = kmap_atomic(page, KM_USER0);
- memset(kaddr, 0, from);
- memset(kaddr + to, 0, PAGE_CACHE_SIZE - to);
- flush_dcache_page(page);
- kunmap_atomic(kaddr, KM_USER0);
+ if (from)
+ zero_user_page(page, 0, from, KM_USER0);
+ if (to < PAGE_CACHE_SIZE)
+ zero_user_page(page, to,
+ PAGE_CACHE_SIZE - to, KM_USER0);
}
}
return 0;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
