From 6fe6900e1e5b6fa9e5c59aa5061f244fe3f467e2 Mon Sep 17 00:00:00 2001 From: Nick Piggin Date: Sun, 6 May 2007 14:49:04 -0700 Subject: [PATCH] mm: make read_cache_page synchronous Ensure pages are uptodate after returning from read_cache_page, which allows us to cut out most of the filesystem-internal PageUptodate calls. I didn't have a great look down the call chains, but this appears to fixes 7 possible use-before uptodate in hfs, 2 in hfsplus, 1 in jfs, a few in ecryptfs, 1 in jffs2, and a possible cleared data overwritten with readpage in block2mtd. All depending on whether the filler is async and/or can return with a !uptodate page. Signed-off-by: Nick Piggin Cc: Hugh Dickins Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/mtd/devices/block2mtd.c | 6 ++-- fs/afs/dir.c | 3 -- fs/afs/mntpt.c | 11 +++----- fs/cramfs/inode.c | 3 +- fs/ecryptfs/mmap.c | 11 +------- fs/ext2/dir.c | 3 -- fs/freevxfs/vxfs_subr.c | 3 -- fs/minix/dir.c | 1 - fs/namei.c | 12 +------- fs/nfs/dir.c | 5 ---- fs/nfs/symlink.c | 6 ---- fs/ntfs/aops.h | 3 +- fs/ntfs/attrib.c | 18 ++---------- fs/ntfs/file.c | 3 +- fs/ntfs/super.c | 30 +++----------------- fs/ocfs2/symlink.c | 7 ----- fs/partitions/check.c | 3 -- fs/reiserfs/xattr.c | 4 --- fs/sysv/dir.c | 10 +------ fs/ufs/dir.c | 6 +--- fs/ufs/util.c | 6 ++-- include/linux/pagemap.h | 11 ++++++++ mm/filemap.c | 49 +++++++++++++++++++++++++-------- mm/swapfile.c | 3 -- 24 files changed, 71 insertions(+), 146 deletions(-) diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c index ce47544dc1..fc4cc8ba9e 100644 --- a/drivers/mtd/devices/block2mtd.c +++ b/drivers/mtd/devices/block2mtd.c @@ -40,13 +40,11 @@ struct block2mtd_dev { static LIST_HEAD(blkmtd_device_list); -static struct page* page_read(struct address_space *mapping, int index) +static struct page *page_read(struct address_space *mapping, int index) { - filler_t *filler = (filler_t*)mapping->a_ops->readpage; - return read_cache_page(mapping, index, filler, NULL); + return read_mapping_page(mapping, index, NULL); } - /* erase a specified part of the device */ static int _block2mtd_erase(struct block2mtd_dev *dev, loff_t to, size_t len) { diff --git a/fs/afs/dir.c b/fs/afs/dir.c index dac5b990c0..0c1e902f17 100644 --- a/fs/afs/dir.c +++ b/fs/afs/dir.c @@ -194,10 +194,7 @@ static struct page *afs_dir_get_page(struct inode *dir, unsigned long index, page = read_mapping_page(dir->i_mapping, index, &file); if (!IS_ERR(page)) { - wait_on_page_locked(page); kmap(page); - if (!PageUptodate(page)) - goto fail; if (!PageChecked(page)) afs_dir_check_page(dir, page); if (PageError(page)) diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c index b905ae37f9..034fcfd4e3 100644 --- a/fs/afs/mntpt.c +++ b/fs/afs/mntpt.c @@ -68,13 +68,11 @@ int afs_mntpt_check_symlink(struct afs_vnode *vnode, struct key *key) } ret = -EIO; - wait_on_page_locked(page); - buf = kmap(page); - if (!PageUptodate(page)) - goto out_free; if (PageError(page)) goto out_free; + buf = kmap(page); + /* examine the symlink's contents */ size = vnode->status.size; _debug("symlink to %*.*s", (int) size, (int) size, buf); @@ -91,8 +89,8 @@ int afs_mntpt_check_symlink(struct afs_vnode *vnode, struct key *key) ret = 0; -out_free: kunmap(page); +out_free: page_cache_release(page); out: _leave(" = %d", ret); @@ -171,8 +169,7 @@ static struct vfsmount *afs_mntpt_do_automount(struct dentry *mntpt) } ret = -EIO; - wait_on_page_locked(page); - if (!PageUptodate(page) || PageError(page)) + if (PageError(page)) goto error; buf = kmap(page); diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c index facd0c89be..3d194a2be3 100644 --- a/fs/cramfs/inode.c +++ b/fs/cramfs/inode.c @@ -180,7 +180,8 @@ static void *cramfs_read(struct super_block *sb, unsigned int offset, unsigned i struct page *page = NULL; if (blocknr + i < devsize) { - page = read_mapping_page(mapping, blocknr + i, NULL); + page = read_mapping_page_async(mapping, blocknr + i, + NULL); /* synchronous error? */ if (IS_ERR(page)) page = NULL; diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index b731b09499..0770c4b66f 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -46,7 +46,6 @@ struct kmem_cache *ecryptfs_lower_page_cache; */ static struct page *ecryptfs_get1page(struct file *file, int index) { - struct page *page; struct dentry *dentry; struct inode *inode; struct address_space *mapping; @@ -54,14 +53,7 @@ static struct page *ecryptfs_get1page(struct file *file, int index) dentry = file->f_path.dentry; inode = dentry->d_inode; mapping = inode->i_mapping; - page = read_cache_page(mapping, index, - (filler_t *)mapping->a_ops->readpage, - (void *)file); - if (IS_ERR(page)) - goto out; - wait_on_page_locked(page); -out: - return page; + return read_mapping_page(mapping, index, (void *)file); } static @@ -233,7 +225,6 @@ int ecryptfs_do_readpage(struct file *file, struct page *page, ecryptfs_printk(KERN_ERR, "Error reading from page cache\n"); goto out; } - wait_on_page_locked(lower_page); page_data = kmap_atomic(page, KM_USER0); lower_page_data = kmap_atomic(lower_page, KM_USER1); memcpy(page_data, lower_page_data, PAGE_CACHE_SIZE); diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c index e89bfc8cf9..1d1e7e30d7 100644 --- a/fs/ext2/dir.c +++ b/fs/ext2/dir.c @@ -161,10 +161,7 @@ static struct page * ext2_get_page(struct inode *dir, unsigned long n) struct address_space *mapping = dir->i_mapping; struct page *page = read_mapping_page(mapping, n, NULL); if (!IS_ERR(page)) { - wait_on_page_locked(page); kmap(page); - if (!PageUptodate(page)) - goto fail; if (!PageChecked(page)) ext2_check_page(page); if (PageError(page)) diff --git a/fs/freevxfs/vxfs_subr.c b/fs/freevxfs/vxfs_subr.c index decac62efe..ed8f0b0dd8 100644 --- a/fs/freevxfs/vxfs_subr.c +++ b/fs/freevxfs/vxfs_subr.c @@ -74,10 +74,7 @@ vxfs_get_page(struct address_space *mapping, u_long n) pp = read_mapping_page(mapping, n, NULL); if (!IS_ERR(pp)) { - wait_on_page_locked(pp); kmap(pp); - if (!PageUptodate(pp)) - goto fail; /** if (!PageChecked(pp)) **/ /** vxfs_check_page(pp); **/ if (PageError(pp)) diff --git a/fs/minix/dir.c b/fs/minix/dir.c index cb4cb571fd..e207cbe709 100644 --- a/fs/minix/dir.c +++ b/fs/minix/dir.c @@ -65,7 +65,6 @@ static struct page * dir_get_page(struct inode *dir, unsigned long n) struct address_space *mapping = dir->i_mapping; struct page *page = read_mapping_page(mapping, n, NULL); if (!IS_ERR(page)) { - wait_on_page_locked(page); kmap(page); if (!PageUptodate(page)) goto fail; diff --git a/fs/namei.c b/fs/namei.c index 880052cadb..94b2f60aec 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2671,19 +2671,9 @@ static char *page_getlink(struct dentry * dentry, struct page **ppage) struct address_space *mapping = dentry->d_inode->i_mapping; page = read_mapping_page(mapping, 0, NULL); if (IS_ERR(page)) - goto sync_fail; - wait_on_page_locked(page); - if (!PageUptodate(page)) - goto async_fail; + return (char*)page; *ppage = page; return kmap(page); - -async_fail: - page_cache_release(page); - return ERR_PTR(-EIO); - -sync_fail: - return (char*)page; } int page_readlink(struct dentry *dentry, char __user *buffer, int buflen) diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index e59fd31c9a..625d8e5fb3 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -334,8 +334,6 @@ int find_dirent_page(nfs_readdir_descriptor_t *desc) status = PTR_ERR(page); goto out; } - if (!PageUptodate(page)) - goto read_error; /* NOTE: Someone else may have changed the READDIRPLUS flag */ desc->page = page; @@ -349,9 +347,6 @@ int find_dirent_page(nfs_readdir_descriptor_t *desc) out: dfprintk(DIRCACHE, "NFS: %s: returns %d\n", __FUNCTION__, status); return status; - read_error: - page_cache_release(page); - return -EIO; } /* diff --git a/fs/nfs/symlink.c b/fs/nfs/symlink.c index f4a0548b9c..bc2821331c 100644 --- a/fs/nfs/symlink.c +++ b/fs/nfs/symlink.c @@ -61,15 +61,9 @@ static void *nfs_follow_link(struct dentry *dentry, struct nameidata *nd) err = page; goto read_failed; } - if (!PageUptodate(page)) { - err = ERR_PTR(-EIO); - goto getlink_read_error; - } nd_set_link(nd, kmap(page)); return page; -getlink_read_error: - page_cache_release(page); read_failed: nd_set_link(nd, err); return NULL; diff --git a/fs/ntfs/aops.h b/fs/ntfs/aops.h index 9393f4b1e2..caecc58f52 100644 --- a/fs/ntfs/aops.h +++ b/fs/ntfs/aops.h @@ -89,9 +89,8 @@ static inline struct page *ntfs_map_page(struct address_space *mapping, struct page *page = read_mapping_page(mapping, index, NULL); if (!IS_ERR(page)) { - wait_on_page_locked(page); kmap(page); - if (PageUptodate(page) && !PageError(page)) + if (!PageError(page)) return page; ntfs_unmap_page(page); return ERR_PTR(-EIO); diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c index 7659cc1929..1c08fefe48 100644 --- a/fs/ntfs/attrib.c +++ b/fs/ntfs/attrib.c @@ -2532,14 +2532,7 @@ int ntfs_attr_set(ntfs_inode *ni, const s64 ofs, const s64 cnt, const u8 val) page = read_mapping_page(mapping, idx, NULL); if (IS_ERR(page)) { ntfs_error(vol->sb, "Failed to read first partial " - "page (sync error, index 0x%lx).", idx); - return PTR_ERR(page); - } - wait_on_page_locked(page); - if (unlikely(!PageUptodate(page))) { - ntfs_error(vol->sb, "Failed to read first partial page " - "(async error, index 0x%lx).", idx); - page_cache_release(page); + "page (error, index 0x%lx).", idx); return PTR_ERR(page); } /* @@ -2602,14 +2595,7 @@ int ntfs_attr_set(ntfs_inode *ni, const s64 ofs, const s64 cnt, const u8 val) page = read_mapping_page(mapping, idx, NULL); if (IS_ERR(page)) { ntfs_error(vol->sb, "Failed to read last partial page " - "(sync error, index 0x%lx).", idx); - return PTR_ERR(page); - } - wait_on_page_locked(page); - if (unlikely(!PageUptodate(page))) { - ntfs_error(vol->sb, "Failed to read last partial page " - "(async error, index 0x%lx).", idx); - page_cache_release(page); + "(error, index 0x%lx).", idx); return PTR_ERR(page); } kaddr = kmap_atomic(page, KM_USER0); diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c index d69c4595cc..dbbac55931 100644 --- a/fs/ntfs/file.c +++ b/fs/ntfs/file.c @@ -236,8 +236,7 @@ do_non_resident_extend: err = PTR_ERR(page); goto init_err_out; } - wait_on_page_locked(page); - if (unlikely(!PageUptodate(page) || PageError(page))) { + if (unlikely(PageError(page))) { page_cache_release(page); err = -EIO; goto init_err_out; diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c index 1594c90b71..2ddde534db 100644 --- a/fs/ntfs/super.c +++ b/fs/ntfs/super.c @@ -2471,7 +2471,6 @@ static s64 get_nr_free_clusters(ntfs_volume *vol) s64 nr_free = vol->nr_clusters; u32 *kaddr; struct address_space *mapping = vol->lcnbmp_ino->i_mapping; - filler_t *readpage = (filler_t*)mapping->a_ops->readpage; struct page *page; pgoff_t index, max_index; @@ -2494,24 +2493,14 @@ static s64 get_nr_free_clusters(ntfs_volume *vol) * Read the page from page cache, getting it from backing store * if necessary, and increment the use count. */ - page = read_cache_page(mapping, index, (filler_t*)readpage, - NULL); + page = read_mapping_page(mapping, index, NULL); /* Ignore pages which errored synchronously. */ if (IS_ERR(page)) { - ntfs_debug("Sync read_cache_page() error. Skipping " + ntfs_debug("read_mapping_page() error. Skipping " "page (index 0x%lx).", index); nr_free -= PAGE_CACHE_SIZE * 8; continue; } - wait_on_page_locked(page); - /* Ignore pages which errored asynchronously. */ - if (!PageUptodate(page)) { - ntfs_debug("Async read_cache_page() error. Skipping " - "page (index 0x%lx).", index); - page_cache_release(page); - nr_free -= PAGE_CACHE_SIZE * 8; - continue; - } kaddr = (u32*)kmap_atomic(page, KM_USER0); /* * For each 4 bytes, subtract the number of set bits. If this @@ -2562,7 +2551,6 @@ static unsigned long __get_nr_free_mft_records(ntfs_volume *vol, { u32 *kaddr; struct address_space *mapping = vol->mftbmp_ino->i_mapping; - filler_t *readpage = (filler_t*)mapping->a_ops->readpage; struct page *page; pgoff_t index; @@ -2576,21 +2564,11 @@ static unsigned long __get_nr_free_mft_records(ntfs_volume *vol, * Read the page from page cache, getting it from backing store * if necessary, and increment the use count. */ - page = read_cache_page(mapping, index, (filler_t*)readpage, - NULL); + page = read_mapping_page(mapping, index, NULL); /* Ignore pages which errored synchronously. */ if (IS_ERR(page)) { - ntfs_debug("Sync read_cache_page() error. Skipping " - "page (index 0x%lx).", index); - nr_free -= PAGE_CACHE_SIZE * 8; - continue; - } - wait_on_page_locked(page); - /* Ignore pages which errored asynchronously. */ - if (!PageUptodate(page)) { - ntfs_debug("Async read_cache_page() error. Skipping " + ntfs_debug("read_mapping_page() error. Skipping " "page (index 0x%lx).", index); - page_cache_release(page); nr_free -= PAGE_CACHE_SIZE * 8; continue; } diff --git a/fs/ocfs2/symlink.c b/fs/ocfs2/symlink.c index 40dc1a51f4..7134007ba2 100644 --- a/fs/ocfs2/symlink.c +++ b/fs/ocfs2/symlink.c @@ -67,16 +67,9 @@ static char *ocfs2_page_getlink(struct dentry * dentry, page = read_mapping_page(mapping, 0, NULL); if (IS_ERR(page)) goto sync_fail; - wait_on_page_locked(page); - if (!PageUptodate(page)) - goto async_fail; *ppage = page; return kmap(page); -async_fail: - page_cache_release(page); - return ERR_PTR(-EIO); - sync_fail: return (char*)page; } diff --git a/fs/partitions/check.c b/fs/partitions/check.c index f01572fca0..6b9dae3f0e 100644 --- a/fs/partitions/check.c +++ b/fs/partitions/check.c @@ -569,9 +569,6 @@ unsigned char *read_dev_sector(struct block_device *bdev, sector_t n, Sector *p) page = read_mapping_page(mapping, (pgoff_t)(n >> (PAGE_CACHE_SHIFT-9)), NULL); if (!IS_ERR(page)) { - wait_on_page_locked(page); - if (!PageUptodate(page)) - goto fail; if (PageError(page)) goto fail; p->v = page; diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c index 2cac56210e..bf6e582145 100644 --- a/fs/reiserfs/xattr.c +++ b/fs/reiserfs/xattr.c @@ -410,11 +410,7 @@ static struct page *reiserfs_get_page(struct inode *dir, unsigned long n) mapping_set_gfp_mask(mapping, GFP_NOFS); page = read_mapping_page(mapping, n, NULL); if (!IS_ERR(page)) { - wait_on_page_locked(page); kmap(page); - if (!PageUptodate(page)) - goto fail; - if (PageError(page)) goto fail; } diff --git a/fs/sysv/dir.c b/fs/sysv/dir.c index ebf7007fa1..e566b387fc 100644 --- a/fs/sysv/dir.c +++ b/fs/sysv/dir.c @@ -54,17 +54,9 @@ static struct page * dir_get_page(struct inode *dir, unsigned long n) { struct address_space *mapping = dir->i_mapping; struct page *page = read_mapping_page(mapping, n, NULL); - if (!IS_ERR(page)) { - wait_on_page_locked(page); + if (!IS_ERR(page)) kmap(page); - if (!PageUptodate(page)) - goto fail; - } return page; - -fail: - dir_put_page(page); - return ERR_PTR(-EIO); } static int sysv_readdir(struct file * filp, void * dirent, filldir_t filldir) diff --git a/fs/ufs/dir.c b/fs/ufs/dir.c index 4890ddf151..4fb8b2e077 100644 --- a/fs/ufs/dir.c +++ b/fs/ufs/dir.c @@ -180,13 +180,9 @@ fail: static struct page *ufs_get_page(struct inode *dir, unsigned long n) { struct address_space *mapping = dir->i_mapping; - struct page *page = read_cache_page(mapping, n, - (filler_t*)mapping->a_ops->readpage, NULL); + struct page *page = read_mapping_page(mapping, n, NULL); if (!IS_ERR(page)) { - wait_on_page_locked(page); kmap(page); - if (!PageUptodate(page)) - goto fail; if (!PageChecked(page)) ufs_check_page(page); if (PageError(page)) diff --git a/fs/ufs/util.c b/fs/ufs/util.c index 17437574f7..84357f1ff0 100644 --- a/fs/ufs/util.c +++ b/fs/ufs/util.c @@ -251,13 +251,11 @@ struct page *ufs_get_locked_page(struct address_space *mapping, page = find_lock_page(mapping, index); if (!page) { - page = read_cache_page(mapping, index, - (filler_t*)mapping->a_ops->readpage, - NULL); + page = read_mapping_page(mapping, index, NULL); if (IS_ERR(page)) { printk(KERN_ERR "ufs_change_blocknr: " - "read_cache_page error: ino %lu, index: %lu\n", + "read_mapping_page error: ino %lu, index: %lu\n", mapping->host->i_ino, index); goto out; } diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index 7a8dcb82a6..b4def5e083 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -95,12 +95,23 @@ static inline struct page *grab_cache_page(struct address_space *mapping, unsign extern struct page * grab_cache_page_nowait(struct address_space *mapping, unsigned long index); +extern struct page * read_cache_page_async(struct address_space *mapping, + unsigned long index, filler_t *filler, + void *data); extern struct page * read_cache_page(struct address_space *mapping, unsigned long index, filler_t *filler, void *data); extern int read_cache_pages(struct address_space *mapping, struct list_head *pages, filler_t *filler, void *data); +static inline struct page *read_mapping_page_async( + struct address_space *mapping, + unsigned long index, void *data) +{ + filler_t *filler = (filler_t *)mapping->a_ops->readpage; + return read_cache_page_async(mapping, index, filler, data); +} + static inline struct page *read_mapping_page(struct address_space *mapping, unsigned long index, void *data) { diff --git a/mm/filemap.c b/mm/filemap.c index 5dfc093ceb..070e7547d5 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1726,7 +1726,7 @@ int generic_file_readonly_mmap(struct file * file, struct vm_area_struct * vma) EXPORT_SYMBOL(generic_file_mmap); EXPORT_SYMBOL(generic_file_readonly_mmap); -static inline struct page *__read_cache_page(struct address_space *mapping, +static struct page *__read_cache_page(struct address_space *mapping, unsigned long index, int (*filler)(void *,struct page*), void *data) @@ -1763,17 +1763,11 @@ repeat: return page; } -/** - * read_cache_page - read into page cache, fill it if needed - * @mapping: the page's address_space - * @index: the page index - * @filler: function to perform the read - * @data: destination for read data - * - * Read into the page cache. If a page already exists, - * and PageUptodate() is not set, try to fill the page. +/* + * Same as read_cache_page, but don't wait for page to become unlocked + * after submitting it to the filler. */ -struct page *read_cache_page(struct address_space *mapping, +struct page *read_cache_page_async(struct address_space *mapping, unsigned long index, int (*filler)(void *,struct page*), void *data) @@ -1804,6 +1798,39 @@ retry: page_cache_release(page); page = ERR_PTR(err); } + out: + mark_page_accessed(page); + return page; +} +EXPORT_SYMBOL(read_cache_page_async); + +/** + * read_cache_page - read into page cache, fill it if needed + * @mapping: the page's address_space + * @index: the page index + * @filler: function to perform the read + * @data: destination for read data + * + * Read into the page cache. If a page already exists, and PageUptodate() is + * not set, try to fill the page then wait for it to become unlocked. + * + * If the page does not get brought uptodate, return -EIO. + */ +struct page *read_cache_page(struct address_space *mapping, + unsigned long index, + int (*filler)(void *,struct page*), + void *data) +{ + struct page *page; + + page = read_cache_page_async(mapping, index, filler, data); + if (IS_ERR(page)) + goto out; + wait_on_page_locked(page); + if (!PageUptodate(page)) { + page_cache_release(page); + page = ERR_PTR(-EIO); + } out: return page; } diff --git a/mm/swapfile.c b/mm/swapfile.c index a2d9bb4e80..acc172cbe3 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -1531,9 +1531,6 @@ asmlinkage long sys_swapon(const char __user * specialfile, int swap_flags) error = PTR_ERR(page); goto bad_swap; } - wait_on_page_locked(page); - if (!PageUptodate(page)) - goto bad_swap; kmap(page); swap_header = page_address(page); -- 2.20.1