Skip to contentSkip to conversation

fs: convert core functions to zero_user_page

PATCH In review Nate Diller 4 participants 18 messages Apr 11, 2007

Subsystems (13)
lore
Share & tools
lore
#0 lore raw

PATCH 1/13 fs: convert core functions to zero_user_page

It's very common for file systems to need to zero part or all of a page, the simplist way is just to use kmap_atomic() and memset(). There's actually a library function in include/linux/highmem.h that does exactly that, but it's confusingly named memclear_highpage_flush(), which is descriptive of *how* it does the work rather than what the *purpose* is. So this patchset renames the function to zero_user_page(), and calls it from the various places that currently open code it.

This first patch introduces the new function call, and converts all the core kernel callsites, both the open-coded ones and the old memclear_highpage_flush() ones. Following this patch is a series of conversions for each file system individually, per AKPM, and finally a patch deprecating the old call. The diffstat below shows the entire patchset.

Compile tested in x86_64.

---

 drivers/block/loop.c                     |    6 ---
 fs/affs/file.c                           |    6 ---
 fs/buffer.c                              |   53 +++++--------------------------
 fs/direct-io.c                           |    8 +---
 fs/ecryptfs/mmap.c                       |   14 +-------
 fs/ext3/inode.c                          |   12 +------
 fs/ext4/inode.c                          |   12 +------
 fs/ext4/writeback.c                      |   12 +------
 fs/gfs2/bmap.c                           |    6 ---
 fs/mpage.c                               |   11 +-----
 fs/nfs/read.c                            |   10 ++---
 fs/nfs/write.c                           |    2 -
 fs/ntfs/aops.c                           |   26 ++-------------
 fs/ntfs/file.c                           |   47 +++++----------------------
 fs/ocfs2/aops.c                          |    5 --
 fs/reiser4/plugin/file/cryptcompress.c   |   19 +----------
 fs/reiser4/plugin/file/file.c            |    6 ---
 fs/reiser4/plugin/item/ctail.c           |    6 ---
 fs/reiser4/plugin/item/extent_file_ops.c |   19 +++--------
 fs/reiser4/plugin/item/tail.c            |    8 +---
 fs/reiserfs/file.c                       |   39 ++++++----------------
 fs/reiserfs/inode.c                      |   13 +------
 fs/xfs/linux-2.6/xfs_lrw.c               |    2 -
 include/linux/highmem.h                  |    7 +++-
 mm/filemap_xip.c                         |    7 ----
 mm/truncate.c                            |    2 -
 26 files changed, 82 insertions(+), 276 deletions(-)

---

drivers/block/loop.c +1 -5
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/drivers/block/loop.c linux-2.6.21-rc6-mm1-test/drivers/block/loop.c
@@ -244,17 +244,13 @@ static int do_lo_send_aops(struct loop_d
 		transfer_result = lo_do_transfer(lo, WRITE, page, offset,
 				bvec->bv_page, bv_offs, size, IV);
 		if (unlikely(transfer_result)) {
-			char *kaddr;
-
 			/*
 			 * The transfer failed, but we still write the data to
 			 * keep prepare/commit calls balanced.
 			 */
 			printk(KERN_ERR "loop: transfer error block %llu\n",
 			       (unsigned long long)index);
-			kaddr = kmap_atomic(page, KM_USER0);
-			memset(kaddr + offset, 0, size);
-			kunmap_atomic(kaddr, KM_USER0);
+			zero_user_page(page, offset, size);
 		}
 		flush_dcache_page(page);
 		ret = aops->commit_write(file, page, offset,
fs/buffer.c +9 -44

138 lines not on this page · raw message

fs/direct-io.c +2 -6
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/direct-io.c linux-2.6.21-rc6-mm1-test/fs/direct-io.c
@@ -867,7 +867,6 @@ static int do_direct_IO(struct dio *dio)
 do_holes:
 			/* Handle holes */
 			if (!buffer_mapped(map_bh)) {
-				char *kaddr;
 				loff_t i_size_aligned;
 
 				/* AKPM: eargh, -ENOTBLK is a hack */
@@ -888,11 +887,8 @@ do_holes:
 					page_cache_release(page);
 					goto out;
 				}
-				kaddr = kmap_atomic(page, KM_USER0);
-				memset(kaddr + (block_in_page << blkbits),
-						0, 1 << blkbits);
-				flush_dcache_page(page);
-				kunmap_atomic(kaddr, KM_USER0);
+				zero_user_page(page, block_in_page << blkbits,
+						1 << blkbits);
 				dio->block_in_file++;
 				block_in_page++;
 				goto next_block;
fs/mpage.c +2 -9
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/mpage.c linux-2.6.21-rc6-mm1-test/fs/mpage.c
@@ -284,11 +284,8 @@ do_mpage_readpage(struct bio *bio, struc
 	}
 
 	if (first_hole != blocks_per_page) {
-		char *kaddr = kmap_atomic(page, KM_USER0);
-		memset(kaddr + (first_hole << blkbits), 0,
+		zero_user_page(page, first_hole << blkbits,
 				PAGE_CACHE_SIZE - (first_hole << blkbits));
-		flush_dcache_page(page);
-		kunmap_atomic(kaddr, KM_USER0);
 		if (first_hole == 0) {
 			SetPageUptodate(page);
 			unlock_page(page);
@@ -586,14 +583,10 @@ page_is_mapped:
 		 * written out to the file."
 		 */
 		unsigned offset = i_size & (PAGE_CACHE_SIZE - 1);
-		char *kaddr;
 
 		if (page->index > end_index || !offset)
 			goto confused;
-		kaddr = kmap_atomic(page, KM_USER0);
-		memset(kaddr + offset, 0, PAGE_CACHE_SIZE - offset);
-		flush_dcache_page(page);
-		kunmap_atomic(kaddr, KM_USER0);
+		zero_user_page(page, offset, PAGE_CACHE_SIZE - offset);
 	}
 
 	/*
include/linux/highmem.h +6 -1
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/include/linux/highmem.h linux-2.6.21-rc6-mm1-test/include/linux/highmem.h
@@ -137,7 +137,7 @@ static inline void clear_highpage(struct
 /*
  * Same but also flushes aliased cache contents to RAM.
  */
-static inline void memclear_highpage_flush(struct page *page, unsigned int offset, unsigned int size)
+static inline void zero_user_page(struct page *page, unsigned int offset, unsigned int size)
 {
 	void *kaddr;
 
@@ -149,6 +149,11 @@ static inline void memclear_highpage_flu
 	kunmap_atomic(kaddr, KM_USER0);
 }
 
+static inline void memclear_highpage_flush(struct page *page, unsigned int offset, unsigned int size)
+{
+	return zero_user_page(page, offset, size);
+}
+
 #ifndef __HAVE_ARCH_COPY_USER_HIGHPAGE
 
 static inline void copy_user_highpage(struct page *to, struct page *from,
mm/filemap_xip.c +1 -6
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/mm/filemap_xip.c linux-2.6.21-rc6-mm1-test/mm/filemap_xip.c
@@ -440,7 +440,6 @@ xip_truncate_page(struct address_space *
 	unsigned blocksize;
 	unsigned length;
 	struct page *page;
-	void *kaddr;
 
 	BUG_ON(!mapping->a_ops->get_xip_page);
 
@@ -464,11 +463,7 @@ xip_truncate_page(struct address_space *
 		else
 			return PTR_ERR(page);
 	}
-	kaddr = kmap_atomic(page, KM_USER0);
-	memset(kaddr + offset, 0, length);
-	kunmap_atomic(kaddr, KM_USER0);
-
-	flush_dcache_page(page);
+	zero_user_page(page, offset, length);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(xip_truncate_page);
mm/truncate.c +1 -1
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/mm/truncate.c linux-2.6.21-rc6-mm1-test/mm/truncate.c
@@ -46,7 +46,7 @@ void do_invalidatepage(struct page *page
 
 static inline void truncate_partial_page(struct page *page, unsigned partial)
 {
-	memclear_highpage_flush(page, partial, PAGE_CACHE_SIZE-partial);
+	zero_user_page(page, partial, PAGE_CACHE_SIZE-partial);
 	if (PagePrivate(page))
 		do_invalidatepage(page, partial);
 }
#1 lore raw

PATCH 2/13 affs: use zero_user_page

Use zero_user_page() instead of open-coding it.

---

fs/affs/file.c +1 -5
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/affs/file.c linux-2.6.21-rc6-mm1-test/fs/affs/file.c
@@ -628,11 +628,7 @@ static int affs_prepare_write_ofs(struct
 			return err;
 	}
 	if (to < PAGE_CACHE_SIZE) {
-		char *kaddr = kmap_atomic(page, KM_USER0);
-
-		memset(kaddr + to, 0, PAGE_CACHE_SIZE - to);
-		flush_dcache_page(page);
-		kunmap_atomic(kaddr, KM_USER0);
+		zero_user_page(page, to, PAGE_CACHE_SIZE - to);
 		if (size > offset + to) {
 			if (size < offset + PAGE_CACHE_SIZE)
 				tmp = size & ~PAGE_CACHE_MASK;
#2 lore raw

PATCH 3/13 ecryptfs: use zero_user_page

In reply to Nate Diller

Use zero_user_page() instead of open-coding it.

---

fs/ecryptfs/mmap.c +3 -11
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/ecryptfs/mmap.c linux-2.6.21-rc6-mm1-test/fs/ecryptfs/mmap.c
@@ -364,18 +364,14 @@ static int fill_zeros_to_end_of_page(str
 {
 	struct inode *inode = page->mapping->host;
 	int end_byte_in_page;
-	char *page_virt;
 
 	if ((i_size_read(inode) / PAGE_CACHE_SIZE) != page->index)
 		goto out;
 	end_byte_in_page = i_size_read(inode) % PAGE_CACHE_SIZE;
 	if (to > end_byte_in_page)
 		end_byte_in_page = to;
-	page_virt = kmap_atomic(page, KM_USER0);
-	memset((page_virt + end_byte_in_page), 0,
-	       (PAGE_CACHE_SIZE - end_byte_in_page));
-	kunmap_atomic(page_virt, KM_USER0);
-	flush_dcache_page(page);
+	zero_user_page(page, end_byte_in_page,
+		PAGE_CACHE_SIZE - end_byte_in_page);
 out:
 	return 0;
 }
@@ -740,7 +736,6 @@ int write_zeros(struct file *file, pgoff
 {
 	int rc = 0;
 	struct page *tmp_page;
-	char *tmp_page_virt;
 
 	tmp_page = ecryptfs_get1page(file, index);
 	if (IS_ERR(tmp_page)) {
@@ -757,10 +752,7 @@ int write_zeros(struct file *file, pgoff
 		page_cache_release(tmp_page);
 		goto out;
 	}
-	tmp_page_virt = kmap_atomic(tmp_page, KM_USER0);
-	memset(((char *)tmp_page_virt + start), 0, num_zeros);
-	kunmap_atomic(tmp_page_virt, KM_USER0);
-	flush_dcache_page(tmp_page);
+	zero_user_page(tmp_page, start, num_zeros);
 	rc = ecryptfs_commit_write(file, tmp_page, start, start + num_zeros);
 	if (rc < 0) {
 		ecryptfs_printk(KERN_ERR, "Error attempting to write zero's "
#3 lore raw

PATCH 4/13 ext3: use zero_user_page

In reply to Nate Diller

Use zero_user_page() instead of open-coding it.

---

fs/ext3/inode.c +2 -10
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/ext3/inode.c linux-2.6.21-rc6-mm1-test/fs/ext3/inode.c
@@ -1767,7 +1767,6 @@ static int ext3_block_truncate_page(hand
 	struct inode *inode = mapping->host;
 	struct buffer_head *bh;
 	int err = 0;
-	void *kaddr;
 
 	blocksize = inode->i_sb->s_blocksize;
 	length = blocksize - (offset & (blocksize - 1));
@@ -1779,10 +1778,7 @@ static int ext3_block_truncate_page(hand
 	 */
 	if (!page_has_buffers(page) && test_opt(inode->i_sb, NOBH) &&
 	     ext3_should_writeback_data(inode) && PageUptodate(page)) {
-		kaddr = kmap_atomic(page, KM_USER0);
-		memset(kaddr + offset, 0, length);
-		flush_dcache_page(page);
-		kunmap_atomic(kaddr, KM_USER0);
+		zero_user_page(page, offset, length);
 		set_page_dirty(page);
 		goto unlock;
 	}
@@ -1835,11 +1831,7 @@ static int ext3_block_truncate_page(hand
 			goto unlock;
 	}
 
-	kaddr = kmap_atomic(page, KM_USER0);
-	memset(kaddr + offset, 0, length);
-	flush_dcache_page(page);
-	kunmap_atomic(kaddr, KM_USER0);
-
+	zero_user_page(page, offset, length);
 	BUFFER_TRACE(bh, "zeroed end of block");
 
 	err = 0;
#4 lore raw

PATCH 5/13 ext4: use zero_user_page

In reply to Nate Diller

Use zero_user_page() instead of open-coding it.

---

fs/ext4/inode.c +2 -10
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/ext4/inode.c linux-2.6.21-rc6-mm1-test/fs/ext4/inode.c
@@ -1791,7 +1791,6 @@ int ext4_block_truncate_page(handle_t *h
 	struct inode *inode = mapping->host;
 	struct buffer_head *bh;
 	int err = 0;
-	void *kaddr;
 
 	if ((EXT4_I(inode)->i_flags & EXT4_EXTENTS_FL) &&
 			test_opt(inode->i_sb, EXTENTS) &&
@@ -1808,10 +1807,7 @@ int ext4_block_truncate_page(handle_t *h
 	 */
 	if (!page_has_buffers(page) && test_opt(inode->i_sb, NOBH) &&
 	     ext4_should_writeback_data(inode) && PageUptodate(page)) {
-		kaddr = kmap_atomic(page, KM_USER0);
-		memset(kaddr + offset, 0, length);
-		flush_dcache_page(page);
-		kunmap_atomic(kaddr, KM_USER0);
+		zero_user_page(page, offset, length);
 		set_page_dirty(page);
 		goto unlock;
 	}
@@ -1864,11 +1860,7 @@ int ext4_block_truncate_page(handle_t *h
 			goto unlock;
 	}
 
-	kaddr = kmap_atomic(page, KM_USER0);
-	memset(kaddr + offset, 0, length);
-	flush_dcache_page(page);
-	kunmap_atomic(kaddr, KM_USER0);
-
+	zero_user_page(page, offset, length);
 	BUFFER_TRACE(bh, "zeroed end of block");
 
 	err = 0;
fs/ext4/writeback.c +2 -10
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/ext4/writeback.c linux-2.6.21-rc6-mm1-test/fs/ext4/writeback.c
@@ -961,7 +961,6 @@ int ext4_wb_writepage(struct page *page,
 	loff_t i_size = i_size_read(inode);
 	pgoff_t end_index = i_size >> PAGE_CACHE_SHIFT;
 	unsigned offset;
-	void *kaddr;
 
 	wb_debug("writepage %lu from inode %lu\n", page->index, inode->i_ino);
 
@@ -1011,10 +1010,7 @@ int ext4_wb_writepage(struct page *page,
 	 * the  page size, the remaining memory is zeroed when mapped, and
 	 * writes to that region are not written out to the file."
 	 */
-	kaddr = kmap_atomic(page, KM_USER0);
-	memset(kaddr + offset, 0, PAGE_CACHE_SIZE - offset);
-	flush_dcache_page(page);
-	kunmap_atomic(kaddr, KM_USER0);
+	zero_user_page(page, offset, PAGE_CACHE_SIZE - offset);
 	return ext4_wb_write_single_page(page, wbc);
 }
 
@@ -1065,7 +1061,6 @@ int ext4_wb_block_truncate_page(handle_t
 	struct inode *inode = mapping->host;
 	struct buffer_head bh, *bhw = &bh;
 	unsigned blocksize, length;
-	void *kaddr;
 	int err = 0;
 
 	wb_debug("partial truncate from %lu on page %lu from inode %lu\n",
@@ -1104,10 +1099,7 @@ int ext4_wb_block_truncate_page(handle_t
 		}
 	}
 
-	kaddr = kmap_atomic(page, KM_USER0);
-	memset(kaddr + offset, 0, length);
-	flush_dcache_page(page);
-	kunmap_atomic(kaddr, KM_USER0);
+	zero_user_page(page, offset, length);
 	SetPageUptodate(page);
 	__set_page_dirty_nobuffers(page);
 
#5 lore raw

Re: [PATCH 5/13] ext4: use zero_user_page

Quoted 7 lines · Nate Diller

Use zero_user_page() instead of open-coding it.

To: Andrew Morton <[email protected]>,
Alexander Viro <[email protected]>
Cc: [email protected], [email protected]

Would have been better to CC the filesystem maintainers directly (which was one of the reasons Andrew wanted per-fs patches so they can be Ack/Nack independently.

Looks good in any case,

Quoted 80 lines

diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/ext4/inode.c linux-2.6.21-rc6-mm1-test/fs/ext4/inode.c

fs/ext4/inode.c +2 -10
@@ -1791,7 +1791,6 @@ int ext4_block_truncate_page(handle_t *h
 	struct inode *inode = mapping->host;
 	struct buffer_head *bh;
 	int err = 0;
-	void *kaddr;
 
 	if ((EXT4_I(inode)->i_flags & EXT4_EXTENTS_FL) &&
 			test_opt(inode->i_sb, EXTENTS) &&
@@ -1808,10 +1807,7 @@ int ext4_block_truncate_page(handle_t *h
 	 */
 	if (!page_has_buffers(page) && test_opt(inode->i_sb, NOBH) &&
 	     ext4_should_writeback_data(inode) && PageUptodate(page)) {
-		kaddr = kmap_atomic(page, KM_USER0);
-		memset(kaddr + offset, 0, length);
-		flush_dcache_page(page);
-		kunmap_atomic(kaddr, KM_USER0);
+		zero_user_page(page, offset, length);
 		set_page_dirty(page);
 		goto unlock;
 	}
@@ -1864,11 +1860,7 @@ int ext4_block_truncate_page(handle_t *h
 			goto unlock;
 	}
 
-	kaddr = kmap_atomic(page, KM_USER0);
-	memset(kaddr + offset, 0, length);
-	flush_dcache_page(page);
-	kunmap_atomic(kaddr, KM_USER0);
-
+	zero_user_page(page, offset, length);
 	BUFFER_TRACE(bh, "zeroed end of block");
 
 	err = 0;

diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/ext4/writeback.c linux-2.6.21-rc6-mm1-test/fs/ext4/writeback.c

fs/ext4/writeback.c +2 -11
@@ -961,7 +961,6 @@ int ext4_wb_writepage(struct page *page,
 	loff_t i_size = i_size_read(inode);
 	pgoff_t end_index = i_size >> PAGE_CACHE_SHIFT;
 	unsigned offset;
-	void *kaddr;
 
 	wb_debug("writepage %lu from inode %lu\n", page->index, inode->i_ino);
 
@@ -1011,10 +1010,7 @@ int ext4_wb_writepage(struct page *page,
 	 * the  page size, the remaining memory is zeroed when mapped, and
 	 * writes to that region are not written out to the file."
 	 */
-	kaddr = kmap_atomic(page, KM_USER0);
-	memset(kaddr + offset, 0, PAGE_CACHE_SIZE - offset);
-	flush_dcache_page(page);
-	kunmap_atomic(kaddr, KM_USER0);
+	zero_user_page(page, offset, PAGE_CACHE_SIZE - offset);
 	return ext4_wb_write_single_page(page, wbc);
 }
 
@@ -1065,7 +1061,6 @@ int ext4_wb_block_truncate_page(handle_t
 	struct inode *inode = mapping->host;
 	struct buffer_head bh, *bhw = &bh;
 	unsigned blocksize, length;
-	void *kaddr;
 	int err = 0;
 
 	wb_debug("partial truncate from %lu on page %lu from inode %lu\n",
@@ -1104,10 +1099,7 @@ int ext4_wb_block_truncate_page(handle_t
 		}
 	}
 
-	kaddr = kmap_atomic(page, KM_USER0);
-	memset(kaddr + offset, 0, length);
-	flush_dcache_page(page);
-	kunmap_atomic(kaddr, KM_USER0);
+	zero_user_page(page, offset, length);
 	SetPageUptodate(page);
 	__set_page_dirty_nobuffers(page);
 
-
Quoted 3 lines

To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

#6 lore raw

PATCH 6/13 gfs2: use zero_user_page

In reply to Nate Diller

Use zero_user_page() instead of open-coding it.

---

fs/gfs2/bmap.c +1 -5
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/gfs2/bmap.c linux-2.6.21-rc6-mm1-test/fs/gfs2/bmap.c
@@ -885,7 +885,6 @@ static int gfs2_block_truncate_page(stru
 	unsigned blocksize, iblock, length, pos;
 	struct buffer_head *bh;
 	struct page *page;
-	void *kaddr;
 	int err;
 
 	page = grab_cache_page(mapping, index);
@@ -933,10 +932,7 @@ static int gfs2_block_truncate_page(stru
 	if (sdp->sd_args.ar_data == GFS2_DATA_ORDERED || gfs2_is_jdata(ip))
 		gfs2_trans_add_bh(ip->i_gl, bh, 0);
 
-	kaddr = kmap_atomic(page, KM_USER0);
-	memset(kaddr + offset, 0, length);
-	flush_dcache_page(page);
-	kunmap_atomic(kaddr, KM_USER0);
+	zero_user_page(page, offset, length);
 
 unlock:
 	unlock_page(page);
#7 lore raw

PATCH 7/13 nfs: use zero_user_page

In reply to Nate Diller

Use zero_user_page() instead of the newly deprecated memclear_highpage_flush().

---

fs/nfs/read.c +5 -5
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/nfs/read.c linux-2.6.21-rc6-mm1-test/fs/nfs/read.c
@@ -79,7 +79,7 @@ void nfs_readdata_release(void *data)
 static
 int nfs_return_empty_page(struct page *page)
 {
-	memclear_highpage_flush(page, 0, PAGE_CACHE_SIZE);
+	zero_user_page(page, 0, PAGE_CACHE_SIZE);
 	SetPageUptodate(page);
 	unlock_page(page);
 	return 0;
@@ -103,10 +103,10 @@ static void nfs_readpage_truncate_uninit
 	pglen = PAGE_CACHE_SIZE - base;
 	for (;;) {
 		if (remainder <= pglen) {
-			memclear_highpage_flush(*pages, base, remainder);
+			zero_user_page(*pages, base, remainder);
 			break;
 		}
-		memclear_highpage_flush(*pages, base, pglen);
+		zero_user_page(*pages, base, pglen);
 		pages++;
 		remainder -= pglen;
 		pglen = PAGE_CACHE_SIZE;
@@ -130,7 +130,7 @@ static int nfs_readpage_async(struct nfs
 		return PTR_ERR(new);
 	}
 	if (len < PAGE_CACHE_SIZE)
-		memclear_highpage_flush(page, len, PAGE_CACHE_SIZE - len);
+		zero_user_page(page, len, PAGE_CACHE_SIZE - len);
 
 	nfs_list_add_request(new, &one_request);
 	nfs_pagein_one(&one_request, inode);
@@ -561,7 +561,7 @@ readpage_async_filler(void *data, struct
 			return PTR_ERR(new);
 	}
 	if (len < PAGE_CACHE_SIZE)
-		memclear_highpage_flush(page, len, PAGE_CACHE_SIZE - len);
+		zero_user_page(page, len, PAGE_CACHE_SIZE - len);
 	nfs_list_add_request(new, desc->head);
 	return 0;
 }
fs/nfs/write.c +1 -1
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/nfs/write.c linux-2.6.21-rc6-mm1-test/fs/nfs/write.c
@@ -169,7 +169,7 @@ static void nfs_mark_uptodate(struct pag
 	if (count != nfs_page_length(page))
 		return;
 	if (count != PAGE_CACHE_SIZE)
-		memclear_highpage_flush(page, count, PAGE_CACHE_SIZE - count);
+		zero_user_page(page, count, PAGE_CACHE_SIZE - count);
 	SetPageUptodate(page);
 }
 
#8 lore raw

PATCH 8/13 ntfs: use zero_user_page

In reply to Nate Diller

Use zero_user_page() instead of open-coding it.

---

fs/ntfs/aops.c +5 -21
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/ntfs/aops.c linux-2.6.21-rc6-mm1-test/fs/ntfs/aops.c
@@ -245,8 +241,7 @@ static int ntfs_read_block(struct page *
 	rl = NULL;
 	nr = i = 0;
 	do {
-		u8 *kaddr;
-		int err;
+		int err = 0;
 
 		if (unlikely(buffer_uptodate(bh)))
 			continue;
@@ -254,7 +249,6 @@ static int ntfs_read_block(struct page *
 			arr[nr++] = bh;
 			continue;
 		}
-		err = 0;
 		bh->b_bdev = vol->sb->s_bdev;
 		/* Is the block within the allowed limits? */
 		if (iblock < lblock) {
@@ -340,10 +334,7 @@ handle_hole:
 		bh->b_blocknr = -1UL;
 		clear_buffer_mapped(bh);
 handle_zblock:
-		kaddr = kmap_atomic(page, KM_USER0);
-		memset(kaddr + i * blocksize, 0, blocksize);
-		kunmap_atomic(kaddr, KM_USER0);
-		flush_dcache_page(page);
+		zero_user_page(page, i * blocksize, blocksize);
 		if (likely(!err))
 			set_buffer_uptodate(bh);
 	} while (i++, iblock++, (bh = bh->b_this_page) != head);
@@ -460,10 +451,7 @@ retry_readpage:
 	 * ok to ignore the compressed flag here.
 	 */
 	if (unlikely(page->index > 0)) {
-		kaddr = kmap_atomic(page, KM_USER0);
-		memset(kaddr, 0, PAGE_CACHE_SIZE);
-		flush_dcache_page(page);
-		kunmap_atomic(kaddr, KM_USER0);
+		zero_user_page(page, 0, PAGE_CACHE_SIZE);
 		goto done;
 	}
 	if (!NInoAttr(ni))
@@ -790,14 +778,9 @@ lock_retry_remap:
 		 * uptodate so it can get discarded by the VM.
 		 */
 		if (err == -ENOENT || lcn == LCN_ENOENT) {
-			u8 *kaddr;
-
 			bh->b_blocknr = -1;
 			clear_buffer_dirty(bh);
-			kaddr = kmap_atomic(page, KM_USER0);
-			memset(kaddr + bh_offset(bh), 0, blocksize);
-			kunmap_atomic(kaddr, KM_USER0);
-			flush_dcache_page(page);
+			zero_user_page(page, bh_offset(bh), blocksize);
 			set_buffer_uptodate(bh);
 			err = 0;
 			continue;
@@ -1422,10 +1405,7 @@ retry_writepage:
 		if (page->index >= (i_size >> PAGE_CACHE_SHIFT)) {
 			/* The page straddles i_size. */
 			unsigned int ofs = i_size & ~PAGE_CACHE_MASK;
-			kaddr = kmap_atomic(page, KM_USER0);
-			memset(kaddr + ofs, 0, PAGE_CACHE_SIZE - ofs);
-			kunmap_atomic(kaddr, KM_USER0);
-			flush_dcache_page(page);
+			zero_user_page(page, ofs, PAGE_CACHE_SIZE - ofs);
 		}
 		/* Handle mst protected attributes. */
 		if (NInoMstProtected(ni))
fs/ntfs/file.c +10 -37

110 lines not on this page · raw message

#9 lore raw

PATCH 9/13 ocfs2: use zero_user_page

In reply to Nate Diller

Use zero_user_page() instead of open-coding it.

---

fs/ocfs2/aops.c +1 -4
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/ocfs2/aops.c linux-2.6.21-rc6-mm1-test/fs/ocfs2/aops.c
@@ -234,10 +234,7 @@ static int ocfs2_readpage(struct file *f
 	 * XXX sys_readahead() seems to get that wrong?
 	 */
 	if (start >= i_size_read(inode)) {
-		char *addr = kmap(page);
-		memset(addr, 0, PAGE_SIZE);
-		flush_dcache_page(page);
-		kunmap(page);
+		zero_user_page(page, 0, PAGE_SIZE);
 		SetPageUptodate(page);
 		ret = 0;
 		goto out_alloc;
#10 lore raw

PATCH 10/13 reiser4: use zero_user_page

In reply to Nate Diller

Use zero_user_page() instead of open-coding it. Also replace the (mostly) redundant zero_page() function.

---

fs/reiser4/plugin/file/cryptcompress.c +3 -16
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/reiser4/plugin/file/cryptcompress.c linux-2.6.21-rc6-mm1-test/fs/reiser4/plugin/file/cryptcompress.c
@@ -1897,7 +1897,6 @@ static int
 write_hole(struct inode *inode, reiser4_cluster_t * clust, loff_t file_off,
 	   loff_t to_file)
 {
-	char *data;
 	int result = 0;
 	unsigned cl_off, cl_count = 0;
 	unsigned to_pg, pg_off;
@@ -1934,10 +1933,7 @@ write_hole(struct inode *inode, reiser4_
 
 		to_pg = min_count(PAGE_CACHE_SIZE - pg_off, cl_count);
 		lock_page(page);
-		data = kmap_atomic(page, KM_USER0);
-		memset(data + pg_off, 0, to_pg);
-		flush_dcache_page(page);
-		kunmap_atomic(data, KM_USER0);
+		zero_user_page(page, pg_off, to_pg);
 		SetPageUptodate(page);
 		unlock_page(page);
 
@@ -2167,7 +2163,6 @@ read_some_cluster_pages(struct inode *in
 
 		if (clust->nr_pages) {
 			int off;
-			char *data;
 			struct page * pg;
 			assert("edward-1419", clust->pages != NULL);
 			pg = clust->pages[clust->nr_pages - 1];
@@ -2175,10 +2170,7 @@ read_some_cluster_pages(struct inode *in
 			off = off_to_pgoff(win->off+win->count+win->delta);
 			if (off) {
 				lock_page(pg);
-				data = kmap_atomic(pg, KM_USER0);
-				memset(data + off, 0, PAGE_CACHE_SIZE - off);
-				flush_dcache_page(pg);
-				kunmap_atomic(data, KM_USER0);
+				zero_user_page(pg, off, PAGE_CACHE_SIZE - off);
 				unlock_page(pg);
 			}
 		}
@@ -2217,20 +2209,15 @@ read_some_cluster_pages(struct inode *in
 		    (count_to_nrpages(inode->i_size) <= pg->index)) {
 			/* .. and appended,
 			   so set zeroes to the rest */
-			char *data;
 			int offset;
 			lock_page(pg);
-			data = kmap_atomic(pg, KM_USER0);
-
 			assert("edward-1260",
 			       count_to_nrpages(win->off + win->count +
 						win->delta) - 1 == i);
 
 			offset =
 			    off_to_pgoff(win->off + win->count + win->delta);
-			memset(data + offset, 0, PAGE_CACHE_SIZE - offset);
-			flush_dcache_page(pg);
-			kunmap_atomic(data, KM_USER0);
+			zero_user_page(pg, offset, PAGE_CACHE_SIZE - offset);
 			unlock_page(pg);
 			/* still not uptodate */
 			break;
fs/reiser4/plugin/file/file.c +1 -5
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/reiser4/plugin/file/file.c linux-2.6.21-rc6-mm1-test/fs/reiser4/plugin/file/file.c
@@ -433,7 +433,6 @@ static int shorten_file(struct inode *in
 	struct page *page;
 	int padd_from;
 	unsigned long index;
-	char *kaddr;
 	unix_file_info_t *uf_info;
 
 	/*
@@ -523,10 +522,7 @@ static int shorten_file(struct inode *in
 
 	lock_page(page);
 	assert("vs-1066", PageLocked(page));
-	kaddr = kmap_atomic(page, KM_USER0);
-	memset(kaddr + padd_from, 0, PAGE_CACHE_SIZE - padd_from);
-	flush_dcache_page(page);
-	kunmap_atomic(kaddr, KM_USER0);
+	zero_user_page(page, padd_from, PAGE_CACHE_SIZE - padd_from);
 	unlock_page(page);
 	page_cache_release(page);
 	/* the below does up(sbinfo->delete_mutex). Do not get confused */
fs/reiser4/plugin/item/ctail.c +1 -5
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/reiser4/plugin/item/ctail.c linux-2.6.21-rc6-mm1-test/fs/reiser4/plugin/item/ctail.c
@@ -627,11 +627,7 @@ int do_readpage_ctail(struct inode * ino
 #endif
 	case FAKE_DISK_CLUSTER:
 		/* fill the page by zeroes */
-		data = kmap_atomic(page, KM_USER0);
-
-		memset(data, 0, PAGE_CACHE_SIZE);
-		flush_dcache_page(page);
-		kunmap_atomic(data, KM_USER0);
+		zero_user_page(page, 0, PAGE_CACHE_SIZE);
 		SetPageUptodate(page);
 		break;
 	case PREP_DISK_CLUSTER:
fs/reiser4/plugin/item/extent_file_ops.c +6 -13
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/reiser4/plugin/item/extent_file_ops.c linux-2.6.21-rc6-mm1-test/fs/reiser4/plugin/item/extent_file_ops.c
@@ -1083,17 +1083,6 @@ ssize_t reiser4_write_extent(struct file
 	return (count - left) ? (count - left) : -EFAULT;
 }
 
-static inline void zero_page(struct page *page)
-{
-	char *kaddr = kmap_atomic(page, KM_USER0);
-
-	memset(kaddr, 0, PAGE_CACHE_SIZE);
-	flush_dcache_page(page);
-	kunmap_atomic(kaddr, KM_USER0);
-	SetPageUptodate(page);
-	unlock_page(page);
-}
-
 int reiser4_do_readpage_extent(reiser4_extent * ext, reiser4_block_nr pos,
 			       struct page *page)
 {
@@ -1115,7 +1104,9 @@ int reiser4_do_readpage_extent(reiser4_e
 		 */
 		j = jfind(mapping, index);
 		if (j == NULL) {
-			zero_page(page);
+			zero_user_page(page, 0, PAGE_CACHE_SIZE);
+			SetPageUptodate(page);
+			unlock_page(page);
 			return 0;
 		}
 		spin_lock_jnode(j);
@@ -1128,7 +1119,9 @@ int reiser4_do_readpage_extent(reiser4_e
 		block = *jnode_get_io_block(j);
 		spin_unlock_jnode(j);
 		if (block == 0) {
-			zero_page(page);
+			zero_user_page(page, 0, PAGE_CACHE_SIZE);
+			SetPageUptodate(page);
+			unlock_page(page);
 			jput(j);
 			return 0;
 		}
fs/reiser4/plugin/item/tail.c +2 -6
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/reiser4/plugin/item/tail.c linux-2.6.21-rc6-mm1-test/fs/reiser4/plugin/item/tail.c
@@ -391,12 +391,8 @@ static int do_readpage_tail(uf_coord_t *
 	}
 
  done:
-	if (mapped != PAGE_CACHE_SIZE) {
-		pagedata = kmap_atomic(page, KM_USER0);
-		memset(pagedata + mapped, 0, PAGE_CACHE_SIZE - mapped);
-		flush_dcache_page(page);
-		kunmap_atomic(pagedata, KM_USER0);
-	}
+	if (mapped != PAGE_CACHE_SIZE)
+		zero_user_page(page, mapped, PAGE_CACHE_SIZE - mapped);
 	SetPageUptodate(page);
  out_unlock_page:
 	unlock_page(page);
#11 lore raw

PATCH 11/13 reiserfs: use zero_user_page

In reply to Nate Diller

Use zero_user_page() instead of open-coding it.

---

fs/reiserfs/file.c +11 -28
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/reiserfs/file.c linux-2.6.21-rc6-mm1-test/fs/reiserfs/file.c
@@ -1059,20 +1059,12 @@ static int reiserfs_prepare_file_region_
 	   maping blocks, since there is none, so we just zero out remaining
 	   parts of first and last pages in write area (if needed) */
 	if ((pos & ~((loff_t) PAGE_CACHE_SIZE - 1)) > inode->i_size) {
-		if (from != 0) {	/* First page needs to be partially zeroed */
-			char *kaddr = kmap_atomic(prepared_pages[0], KM_USER0);
-			memset(kaddr, 0, from);
-			kunmap_atomic(kaddr, KM_USER0);
-			flush_dcache_page(prepared_pages[0]);
-		}
-		if (to != PAGE_CACHE_SIZE) {	/* Last page needs to be partially zeroed */
-			char *kaddr =
-			    kmap_atomic(prepared_pages[num_pages - 1],
-					KM_USER0);
-			memset(kaddr + to, 0, PAGE_CACHE_SIZE - to);
-			kunmap_atomic(kaddr, KM_USER0);
-			flush_dcache_page(prepared_pages[num_pages - 1]);
-		}
+		if (from != 0)		/* First page needs to be partially zeroed */
+			zero_user_page(prepared_pages[0], 0, from);
+
+		if (to != PAGE_CACHE_SIZE)	/* Last page needs to be partially zeroed */
+			zero_user_page(prepared_pages[num_pages-1], to,
+					PAGE_CACHE_SIZE - to);
 
 		/* Since all blocks are new - use already calculated value */
 		return blocks;
@@ -1199,13 +1191,9 @@ static int reiserfs_prepare_file_region_
 					ll_rw_block(READ, 1, &bh);
 					*wait_bh++ = bh;
 				} else {	/* Not mapped, zero it */
-					char *kaddr =
-					    kmap_atomic(prepared_pages[0],
-							KM_USER0);
-					memset(kaddr + block_start, 0,
-					       from - block_start);
-					kunmap_atomic(kaddr, KM_USER0);
-					flush_dcache_page(prepared_pages[0]);
+					zero_user_page(prepared_pages[0],
+						       block_start,
+						       from - block_start);
 					set_buffer_uptodate(bh);
 				}
 			}
@@ -1237,13 +1225,8 @@ static int reiserfs_prepare_file_region_
 					ll_rw_block(READ, 1, &bh);
 					*wait_bh++ = bh;
 				} else {	/* Not mapped, zero it */
-					char *kaddr =
-					    kmap_atomic(prepared_pages
-							[num_pages - 1],
-							KM_USER0);
-					memset(kaddr + to, 0, block_end - to);
-					kunmap_atomic(kaddr, KM_USER0);
-					flush_dcache_page(prepared_pages[num_pages - 1]);
+					zero_user_page(prepared_pages[num_pages-1],
+							to, block_end - to);
 					set_buffer_uptodate(bh);
 				}
 			}
fs/reiserfs/inode.c +2 -11
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/reiserfs/inode.c linux-2.6.21-rc6-mm1-test/fs/reiserfs/inode.c
@@ -2148,13 +2148,8 @@ int reiserfs_truncate_file(struct inode 
 		length = offset & (blocksize - 1);
 		/* if we are not on a block boundary */
 		if (length) {
-			char *kaddr;
-
 			length = blocksize - length;
-			kaddr = kmap_atomic(page, KM_USER0);
-			memset(kaddr + offset, 0, length);
-			flush_dcache_page(page);
-			kunmap_atomic(kaddr, KM_USER0);
+			zero_user_page(page, offset, length);
 			if (buffer_mapped(bh) && bh->b_blocknr != 0) {
 				mark_buffer_dirty(bh);
 			}
@@ -2370,7 +2365,6 @@ static int reiserfs_write_full_page(stru
 	 ** last byte in the file
 	 */
 	if (page->index >= end_index) {
-		char *kaddr;
 		unsigned last_offset;
 
 		last_offset = inode->i_size & (PAGE_CACHE_SIZE - 1);
@@ -2379,10 +2373,7 @@ static int reiserfs_write_full_page(stru
 			unlock_page(page);
 			return 0;
 		}
-		kaddr = kmap_atomic(page, KM_USER0);
-		memset(kaddr + last_offset, 0, PAGE_CACHE_SIZE - last_offset);
-		flush_dcache_page(page);
-		kunmap_atomic(kaddr, KM_USER0);
+		zero_user_page(page, last_offset, PAGE_CACHE_SIZE - last_offset);
 	}
 	bh = head;
 	block = page->index << (PAGE_CACHE_SHIFT - s->s_blocksize_bits);
#12 lore raw

PATCH 12/13 xfs: use zero_user_page

In reply to Nate Diller

Use zero_user_page() instead of the newly deprecated memclear_highpage_flush().

---

fs/xfs/linux-2.6/xfs_lrw.c +1 -1
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/xfs/linux-2.6/xfs_lrw.c linux-2.6.21-rc6-mm1-test/fs/xfs/linux-2.6/xfs_lrw.c
@@ -159,7 +159,7 @@ xfs_iozero(
 		if (status)
 			goto unlock;
 
-		memclear_highpage_flush(page, offset, bytes);
+		zero_user_page(page, offset, bytes);
 
 		status = mapping->a_ops->commit_write(NULL, page, offset,
 							offset + bytes);
#13 lore raw

PATCH 13/13 fs: deprecate memclear_highpage_flush

In reply to Nate Diller

Now that all the in-tree users are converted over to zero_user_page(), deprecate the old memclear_highpage_flush() call.

---

include/linux/highmem.h +2 -0
diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/include/linux/highmem.h linux-2.6.21-rc6-mm1-test/include/linux/highmem.h
@@ -149,6 +149,8 @@ static inline void zero_user_page(struct
 	kunmap_atomic(kaddr, KM_USER0);
 }
 
+static void memclear_highpage_flush(struct page *page, unsigned int offset,
+					unsigned int size) __deprecated;
 static inline void memclear_highpage_flush(struct page *page, unsigned int offset, unsigned int size)
 {
 	return zero_user_page(page, offset, size);
maintainer
#14 lore raw

Re: [PATCH 13/13] fs: deprecate memclear_highpage_flush

Quoted 11 lines · On Tue, 10 Apr 2007 20:36:00 -0700 Nate Diller <[email protected]> wrote:

Now that all the in-tree users are converted over to zero_user_page(), deprecate the old memclear_highpage_flush() call.

---

diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/include/linux/highmem.h linux-2.6.21-rc6-mm1-test/include/linux/highmem.h

include/linux/highmem.h static inline void zero_user_page(struct · line 156

Quoted 8 lines
include/linux/highmem.h +2 -0
@@ -149,6 +149,8 @@ static inline void zero_user_page(struct
 	kunmap_atomic(kaddr, KM_USER0);
 }
 
+static void memclear_highpage_flush(struct page *page, unsigned int offset,
+					unsigned int size) __deprecated;
 static inline void memclear_highpage_flush(struct page *page, unsigned int offset, unsigned int size)
 {
 	return zero_user_page(page, offset, size);

oh, there it is.

one can stick the __deprecated at the end of the definition, actually.

maintainer
#15 lore raw
In reply to Nate Diller
Quoted 8 lines · On Tue, 10 Apr 2007 20:36:00 -0700 Nate Diller <[email protected]> wrote:

It's very common for file systems to need to zero part or all of a page, the simplist way is just to use kmap_atomic() and memset(). There's actually a library function in include/linux/highmem.h that does exactly that, but it's confusingly named memclear_highpage_flush(), which is descriptive of *how* it does the work rather than what the *purpose* is. So this patchset renames the function to zero_user_page(), and calls it from the various places that currently open code it.

Quoted 5 lines

This first patch introduces the new function call, and converts all the core kernel callsites, both the open-coded ones and the old memclear_highpage_flush() ones. Following this patch is a series of conversions for each file system individually, per AKPM, and finally a patch deprecating the old call.

For the reasons Anton identified, I think it is better design while we're here to force callers to pass in the kmap-type which they wish to use for the atomic kmap. It makes the programmer think about what he wants to happen. The price of getting this wrong tends to be revoltingly rare file corruption.

But we cannot make this change in the obvious fashion, because the KM_FOO identifiers are undefined if CONFIG_HIGHMEM=n. So

	zero_user_page(page, 1, 2, KM_USER0);

won't compile on non-highmem.

So we are forced to use a macro, like below.

Also, you forgot to mark memclear_highpage_flush() __deprecated.

And I'm surprised that this:

+static inline void memclear_highpage_flush(struct page *page, unsigned int offset, unsigned int size) +{
+ return zero_user_page(page, offset, size);
+}

compiled. zero_user_page() returns void...

 drivers/block/loop.c    |    2 +-
 fs/buffer.c             |   21 ++++++++++++---------
 fs/direct-io.c          |    2 +-
 fs/mpage.c              |    6 ++++--
 include/linux/highmem.h |   29 +++++++++++++++++------------
 mm/filemap_xip.c        |    2 +-
 6 files changed, 36 insertions(+), 26 deletions(-)
drivers/block/loop.c renamed from drivers/block/loop.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type +1 -1
diff -puN drivers/block/loop.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type drivers/block/loop.c
@@ -250,7 +250,7 @@ static int do_lo_send_aops(struct loop_d
 			 */
 			printk(KERN_ERR "loop: transfer error block %llu\n",
 			       (unsigned long long)index);
-			zero_user_page(page, offset, size);
+			zero_user_page(page, offset, size, KM_USER0);
 		}
 		flush_dcache_page(page);
 		ret = aops->commit_write(file, page, offset,
fs/buffer.c renamed from fs/buffer.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type +12 -9
diff -puN fs/buffer.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type fs/buffer.c
@@ -1855,7 +1855,7 @@ static int __block_prepare_write(struct 
 			break;
 		if (buffer_new(bh)) {
 			clear_buffer_new(bh);
-			zero_user_page(page, block_start, bh->b_size);
+			zero_user_page(page, block_start, bh->b_size, KM_USER0);
 			set_buffer_uptodate(bh);
 			mark_buffer_dirty(bh);
 		}
@@ -1943,7 +1943,8 @@ int block_read_full_page(struct page *pa
 					SetPageError(page);
 			}
 			if (!buffer_mapped(bh)) {
-				zero_user_page(page, i * blocksize, blocksize);
+				zero_user_page(page, i * blocksize, blocksize,
+						KM_USER0);
 				if (!err)
 					set_buffer_uptodate(bh);
 				continue;
@@ -2107,7 +2108,8 @@ int cont_prepare_write(struct page *page
 						PAGE_CACHE_SIZE, get_block);
 		if (status)
 			goto out_unmap;
-		zero_user_page(page, zerofrom, PAGE_CACHE_SIZE-zerofrom);
+		zero_user_page(page, zerofrom, PAGE_CACHE_SIZE - zerofrom,
+				KM_USER0);
 		generic_commit_write(NULL, new_page, zerofrom, PAGE_CACHE_SIZE);
 		unlock_page(new_page);
 		page_cache_release(new_page);
@@ -2134,7 +2136,7 @@ int cont_prepare_write(struct page *page
 	if (status)
 		goto out1;
 	if (zerofrom < offset) {
-		zero_user_page(page, zerofrom, offset-zerofrom);
+		zero_user_page(page, zerofrom, offset - zerofrom, KM_USER0);
 		__block_commit_write(inode, page, zerofrom, offset);
 	}
 	return 0;
@@ -2333,7 +2335,7 @@ failed:
 	 * Error recovery is pretty slack.  Clear the page and mark it dirty
 	 * so we'll later zero out any blocks which _were_ allocated.
 	 */
-	zero_user_page(page, 0, PAGE_CACHE_SIZE);
+	zero_user_page(page, 0, PAGE_CACHE_SIZE, KM_USER0);
 	SetPageUptodate(page);
 	set_page_dirty(page);
 	return ret;
@@ -2402,7 +2404,7 @@ int nobh_writepage(struct page *page, ge
 	 * the  page size, the remaining memory is zeroed when mapped, and
 	 * writes to that region are not written out to the file."
 	 */
-	zero_user_page(page, offset, PAGE_CACHE_SIZE - offset);
+	zero_user_page(page, offset, PAGE_CACHE_SIZE - offset, KM_USER0);
 out:
 	ret = mpage_writepage(page, get_block, wbc);
 	if (ret == -EAGAIN)
@@ -2436,7 +2438,8 @@ int nobh_truncate_page(struct address_sp
 	to = (offset + blocksize) & ~(blocksize - 1);
 	ret = a_ops->prepare_write(NULL, page, offset, to);
 	if (ret == 0) {
-		zero_user_page(page, offset, PAGE_CACHE_SIZE - offset);
+		zero_user_page(page, offset, PAGE_CACHE_SIZE - offset,
+				KM_USER0);
 		/*
 		 * It would be more correct to call aops->commit_write()
 		 * here, but this is more efficient.
@@ -2515,7 +2518,7 @@ int block_truncate_page(struct address_s
 			goto unlock;
 	}
 
-	zero_user_page(page, offset, length);
+	zero_user_page(page, offset, length, KM_USER0);
 	mark_buffer_dirty(bh);
 	err = 0;
 
@@ -2561,7 +2564,7 @@ int block_write_full_page(struct page *p
 	 * the  page size, the remaining memory is zeroed when mapped, and
 	 * writes to that region are not written out to the file."
 	 */
-	zero_user_page(page, offset, PAGE_CACHE_SIZE - offset);
+	zero_user_page(page, offset, PAGE_CACHE_SIZE - offset, KM_USER0);
 	return __block_write_full_page(inode, page, get_block, wbc);
 }
 
fs/direct-io.c renamed from fs/direct-io.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type +1 -1
diff -puN fs/direct-io.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type fs/direct-io.c
@@ -888,7 +888,7 @@ do_holes:
 					goto out;
 				}
 				zero_user_page(page, block_in_page << blkbits,
-						1 << blkbits);
+						1 << blkbits, KM_USER0);
 				dio->block_in_file++;
 				block_in_page++;
 				goto next_block;
fs/mpage.c renamed from fs/mpage.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type +4 -2
diff -puN fs/mpage.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type fs/mpage.c
@@ -285,7 +285,8 @@ do_mpage_readpage(struct bio *bio, struc
 
 	if (first_hole != blocks_per_page) {
 		zero_user_page(page, first_hole << blkbits,
-				PAGE_CACHE_SIZE - (first_hole << blkbits));
+				PAGE_CACHE_SIZE - (first_hole << blkbits),
+				KM_USER0);
 		if (first_hole == 0) {
 			SetPageUptodate(page);
 			unlock_page(page);
@@ -584,7 +585,8 @@ page_is_mapped:
 
 		if (page->index > end_index || !offset)
 			goto confused;
-		zero_user_page(page, offset, PAGE_CACHE_SIZE - offset);
+		zero_user_page(page, offset, PAGE_CACHE_SIZE - offset,
+				KM_USER0);
 	}
 
 	/*
include/linux/highmem.h renamed from include/linux/highmem.h~fs-convert-core-functions-to-zero_user_page-pass-kmap-type +17 -12
diff -puN include/linux/highmem.h~fs-convert-core-functions-to-zero_user_page-pass-kmap-type include/linux/highmem.h
@@ -136,22 +136,27 @@ static inline void clear_highpage(struct
 
 /*
  * Same but also flushes aliased cache contents to RAM.
+ *
+ * This must be a macro because KM_USER0 and friends aren't defined if
+ * !CONFIG_HIGHMEM
  */
-static inline void zero_user_page(struct page *page, unsigned int offset, unsigned int size)
-{
-	void *kaddr;
-
-	BUG_ON(offset + size > PAGE_SIZE);
+#define zero_user_page(page, offset, size, km_type)		\
+	do {							\
+		void *kaddr;					\
+								\
+		BUG_ON(offset + size > PAGE_SIZE);		\
+								\
+		kaddr = kmap_atomic(page, km_type);		\
+		memset((char *)kaddr + offset, 0, size);	\
+		flush_dcache_page(page);			\
+		kunmap_atomic(kaddr, km_type);			\
+	} while (0)
 
-	kaddr = kmap_atomic(page, KM_USER0);
-	memset((char *)kaddr + offset, 0, size);
-	flush_dcache_page(page);
-	kunmap_atomic(kaddr, KM_USER0);
-}
 
-static inline void memclear_highpage_flush(struct page *page, unsigned int offset, unsigned int size)
+static inline void memclear_highpage_flush(struct page *page,
+			unsigned int offset, unsigned int size) __deprecated
 {
-	return zero_user_page(page, offset, size);
+	zero_user_page(page, offset, size);
 }
 
 #ifndef __HAVE_ARCH_COPY_USER_HIGHPAGE
mm/filemap_xip.c renamed from mm/filemap_xip.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type +1 -1
diff -puN mm/filemap_xip.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type mm/filemap_xip.c
@@ -463,7 +463,7 @@ xip_truncate_page(struct address_space *
 		else
 			return PTR_ERR(page);
 	}
-	zero_user_page(page, offset, length);
+	zero_user_page(page, offset, length, KM_USER0);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(xip_truncate_page);

_

#16 lore raw
Quoted 16 lines · Andrew Morton
Quoted 8 lines · On Tue, 10 Apr 2007 20:36:00 -0700 Nate Diller <[email protected]> wrote:

It's very common for file systems to need to zero part or all of a page, the simplist way is just to use kmap_atomic() and memset(). There's actually a library function in include/linux/highmem.h that does exactly that, but it's confusingly named memclear_highpage_flush(), which is descriptive of *how* it does the work rather than what the *purpose* is. So this patchset renames the function to zero_user_page(), and calls it from the various places that currently open code it.

Quoted 5 lines

This first patch introduces the new function call, and converts all the core kernel callsites, both the open-coded ones and the old memclear_highpage_flush() ones. Following this patch is a series of conversions for each file system individually, per AKPM, and finally a patch deprecating the old call.

Quoted 4 lines

For the reasons Anton identified, I think it is better design while we're here to force callers to pass in the kmap-type which they wish to use for the atomic kmap. It makes the programmer think about what he wants to happen. The price of getting this wrong tends to be revoltingly rare file corruption.

yeah, I actually agree with you, on thinking about it. Thanks for doing the conversion :)

Quoted 10 lines

But we cannot make this change in the obvious fashion, because the KM_FOO identifiers are undefined if CONFIG_HIGHMEM=n. So

        zero_user_page(page, 1, 2, KM_USER0);

won't compile on non-highmem.

So we are forced to use a macro, like below.

Also, you forgot to mark memclear_highpage_flush() __deprecated.

that follows in a later patch ... for some reason I had trouble compiling using your notation, and i had to add a function prototype with the __deprecated flag. shrug.

Quoted 9 lines

And I'm surprised that this:

(quoted diff) +4 -0
+static inline void memclear_highpage_flush(struct page *page, unsigned int offset, unsigned int size)
+{
+       return zero_user_page(page, offset, size);
+}

compiled. zero_user_page() returns void...

it's funny, it didn't even warn about it. also it seems your version below is incomplete ... shouldn't it read:

+static inline void memclear_highpage_flush(struct page *page,
+ unsigned int offset, unsigned int size) __deprecated
{
- return zero_user_page(page, offset, size);
+ zero_user_page(page, offset, size, KM_USER0);
}

NATE

Quoted 201 lines
 drivers/block/loop.c    |    2 +-
 fs/buffer.c             |   21 ++++++++++++---------
 fs/direct-io.c          |    2 +-
 fs/mpage.c              |    6 ++++--
 include/linux/highmem.h |   29 +++++++++++++++++------------
 mm/filemap_xip.c        |    2 +-
 6 files changed, 36 insertions(+), 26 deletions(-)

diff -puN drivers/block/loop.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type drivers/block/loop.c

drivers/block/loop.c renamed from drivers/block/loop.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type +1 -1
@@ -250,7 +250,7 @@ static int do_lo_send_aops(struct loop_d
                         */
                        printk(KERN_ERR "loop: transfer error block %llu\n",
                               (unsigned long long)index);
-                       zero_user_page(page, offset, size);
+                       zero_user_page(page, offset, size, KM_USER0);
                }
                flush_dcache_page(page);
                ret = aops->commit_write(file, page, offset,

diff -puN fs/buffer.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type fs/buffer.c

fs/buffer.c renamed from fs/buffer.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type +12 -9
@@ -1855,7 +1855,7 @@ static int __block_prepare_write(struct
                        break;
                if (buffer_new(bh)) {
                        clear_buffer_new(bh);
-                       zero_user_page(page, block_start, bh->b_size);
+                       zero_user_page(page, block_start, bh->b_size, KM_USER0);
                        set_buffer_uptodate(bh);
                        mark_buffer_dirty(bh);
                }
@@ -1943,7 +1943,8 @@ int block_read_full_page(struct page *pa
                                        SetPageError(page);
                        }
                        if (!buffer_mapped(bh)) {
-                               zero_user_page(page, i * blocksize, blocksize);
+                               zero_user_page(page, i * blocksize, blocksize,
+                                               KM_USER0);
                                if (!err)
                                        set_buffer_uptodate(bh);
                                continue;
@@ -2107,7 +2108,8 @@ int cont_prepare_write(struct page *page
                                                PAGE_CACHE_SIZE, get_block);
                if (status)
                        goto out_unmap;
-               zero_user_page(page, zerofrom, PAGE_CACHE_SIZE-zerofrom);
+               zero_user_page(page, zerofrom, PAGE_CACHE_SIZE - zerofrom,
+                               KM_USER0);
                generic_commit_write(NULL, new_page, zerofrom, PAGE_CACHE_SIZE);
                unlock_page(new_page);
                page_cache_release(new_page);
@@ -2134,7 +2136,7 @@ int cont_prepare_write(struct page *page
        if (status)
                goto out1;
        if (zerofrom < offset) {
-               zero_user_page(page, zerofrom, offset-zerofrom);
+               zero_user_page(page, zerofrom, offset - zerofrom, KM_USER0);
                __block_commit_write(inode, page, zerofrom, offset);
        }
        return 0;
@@ -2333,7 +2335,7 @@ failed:
         * Error recovery is pretty slack.  Clear the page and mark it dirty
         * so we'll later zero out any blocks which _were_ allocated.
         */
-       zero_user_page(page, 0, PAGE_CACHE_SIZE);
+       zero_user_page(page, 0, PAGE_CACHE_SIZE, KM_USER0);
        SetPageUptodate(page);
        set_page_dirty(page);
        return ret;
@@ -2402,7 +2404,7 @@ int nobh_writepage(struct page *page, ge
         * the  page size, the remaining memory is zeroed when mapped, and
         * writes to that region are not written out to the file."
         */
-       zero_user_page(page, offset, PAGE_CACHE_SIZE - offset);
+       zero_user_page(page, offset, PAGE_CACHE_SIZE - offset, KM_USER0);
 out:
        ret = mpage_writepage(page, get_block, wbc);
        if (ret == -EAGAIN)
@@ -2436,7 +2438,8 @@ int nobh_truncate_page(struct address_sp
        to = (offset + blocksize) & ~(blocksize - 1);
        ret = a_ops->prepare_write(NULL, page, offset, to);
        if (ret == 0) {
-               zero_user_page(page, offset, PAGE_CACHE_SIZE - offset);
+               zero_user_page(page, offset, PAGE_CACHE_SIZE - offset,
+                               KM_USER0);
                /*
                 * It would be more correct to call aops->commit_write()
                 * here, but this is more efficient.
@@ -2515,7 +2518,7 @@ int block_truncate_page(struct address_s
                        goto unlock;
        }
 
-       zero_user_page(page, offset, length);
+       zero_user_page(page, offset, length, KM_USER0);
        mark_buffer_dirty(bh);
        err = 0;
 
@@ -2561,7 +2564,7 @@ int block_write_full_page(struct page *p
         * the  page size, the remaining memory is zeroed when mapped, and
         * writes to that region are not written out to the file."
         */
-       zero_user_page(page, offset, PAGE_CACHE_SIZE - offset);
+       zero_user_page(page, offset, PAGE_CACHE_SIZE - offset, KM_USER0);
        return __block_write_full_page(inode, page, get_block, wbc);
 }
 

diff -puN fs/direct-io.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type fs/direct-io.c

fs/direct-io.c renamed from fs/direct-io.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type +1 -1
@@ -888,7 +888,7 @@ do_holes:
                                        goto out;
                                }
                                zero_user_page(page, block_in_page << blkbits,
-                                               1 << blkbits);
+                                               1 << blkbits, KM_USER0);
                                dio->block_in_file++;
                                block_in_page++;
                                goto next_block;

diff -puN fs/mpage.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type fs/mpage.c

fs/mpage.c renamed from fs/mpage.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type +4 -2
@@ -285,7 +285,8 @@ do_mpage_readpage(struct bio *bio, struc
 
        if (first_hole != blocks_per_page) {
                zero_user_page(page, first_hole << blkbits,
-                               PAGE_CACHE_SIZE - (first_hole << blkbits));
+                               PAGE_CACHE_SIZE - (first_hole << blkbits),
+                               KM_USER0);
                if (first_hole == 0) {
                        SetPageUptodate(page);
                        unlock_page(page);
@@ -584,7 +585,8 @@ page_is_mapped:
 
                if (page->index > end_index || !offset)
                        goto confused;
-               zero_user_page(page, offset, PAGE_CACHE_SIZE - offset);
+               zero_user_page(page, offset, PAGE_CACHE_SIZE - offset,
+                               KM_USER0);
        }
 
        /*

diff -puN include/linux/highmem.h~fs-convert-core-functions-to-zero_user_page-pass-kmap-type include/linux/highmem.h

include/linux/highmem.h renamed from include/linux/highmem.h~fs-convert-core-functions-to-zero_user_page-pass-kmap-type +17 -12
@@ -136,22 +136,27 @@ static inline void clear_highpage(struct
 
 /*
  * Same but also flushes aliased cache contents to RAM.
+ *
+ * This must be a macro because KM_USER0 and friends aren't defined if
+ * !CONFIG_HIGHMEM
  */
-static inline void zero_user_page(struct page *page, unsigned int offset, unsigned int size)
-{
-       void *kaddr;
-
-       BUG_ON(offset + size > PAGE_SIZE);
+#define zero_user_page(page, offset, size, km_type)            \
+       do {                                                    \
+               void *kaddr;                                    \
+                                                               \
+               BUG_ON(offset + size > PAGE_SIZE);              \
+                                                               \
+               kaddr = kmap_atomic(page, km_type);             \
+               memset((char *)kaddr + offset, 0, size);        \
+               flush_dcache_page(page);                        \
+               kunmap_atomic(kaddr, km_type);                  \
+       } while (0)
 
-       kaddr = kmap_atomic(page, KM_USER0);
-       memset((char *)kaddr + offset, 0, size);
-       flush_dcache_page(page);
-       kunmap_atomic(kaddr, KM_USER0);
-}
 
-static inline void memclear_highpage_flush(struct page *page, unsigned int offset, unsigned int size)
+static inline void memclear_highpage_flush(struct page *page,
+                       unsigned int offset, unsigned int size) __deprecated
 {
-       return zero_user_page(page, offset, size);
+       zero_user_page(page, offset, size);
 }
 
 #ifndef __HAVE_ARCH_COPY_USER_HIGHPAGE

diff -puN mm/filemap_xip.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type mm/filemap_xip.c

mm/filemap_xip.c renamed from mm/filemap_xip.c~fs-convert-core-functions-to-zero_user_page-pass-kmap-type +1 -1
@@ -463,7 +463,7 @@ xip_truncate_page(struct address_space *
                else
                        return PTR_ERR(page);
        }
-       zero_user_page(page, offset, length);
+       zero_user_page(page, offset, length, KM_USER0);
        return 0;
 }
 EXPORT_SYMBOL_GPL(xip_truncate_page);

_

#17 lore raw
In reply to Andrew Morton
Quoted 9 lines · Andrew Morton

And I'm surprised that this:

(quoted diff) +4 -0
+static inline void memclear_highpage_flush(struct page *page, unsigned int offset, unsigned int size)
+{
+	return zero_user_page(page, offset, size);
+}

compiled. zero_user_page() returns void...

As does memclear_highpage_flush(). Some of my code looks like:
void some_func(...)

{
	if (foo)
		return do_foo(...);
	if (bar)
		return do_bar(...);
	...
}

do_foo() and do_bar() also return void. Saves an extra line for the return statment and the brackets.

Doesn't help in the code you quoted, of course.

Jörn

Signature · 7 lines
Measure. Don't tune for speed until you've measured, and even then
don't unless one part of the code overwhelms the rest.
-- Rob Pike
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html