From 2b69c8280c8b29cdeb78b8e92e20ed35f730d319 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 5 Jul 2017 15:26:48 -0400 Subject: [PATCH 1/4] mm: drop "wait" parameter from write_one_page() The callers all set it to 1. Also, make it clear that this function will not set any sort of AS_* error, and that the caller must do so if necessary. No existing caller uses this on normal files, so none of them need it. Also, add __must_check here since, in general, the callers need to handle an error here in some fashion. Link: http://lkml.kernel.org/r/20170525103303.6524-1-jlayton@redhat.com Signed-off-by: Jeff Layton Reviewed-by: Ross Zwisler Reviewed-by: Jan Kara Reviewed-by: Matthew Wilcox Reviewed-by: Christoph Hellwig Signed-off-by: Andrew Morton --- fs/exofs/dir.c | 2 +- fs/ext2/dir.c | 2 +- fs/jfs/jfs_metapage.c | 4 ++-- fs/minix/dir.c | 2 +- fs/sysv/dir.c | 2 +- fs/ufs/dir.c | 2 +- include/linux/mm.h | 2 +- mm/page-writeback.c | 14 +++++++------- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/fs/exofs/dir.c b/fs/exofs/dir.c index 8eeb694332fe..98233a97b7b8 100644 --- a/fs/exofs/dir.c +++ b/fs/exofs/dir.c @@ -72,7 +72,7 @@ static int exofs_commit_chunk(struct page *page, loff_t pos, unsigned len) set_page_dirty(page); if (IS_DIRSYNC(dir)) - err = write_one_page(page, 1); + err = write_one_page(page); else unlock_page(page); diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c index d9650c9508e4..e2709695b177 100644 --- a/fs/ext2/dir.c +++ b/fs/ext2/dir.c @@ -100,7 +100,7 @@ static int ext2_commit_chunk(struct page *page, loff_t pos, unsigned len) } if (IS_DIRSYNC(dir)) { - err = write_one_page(page, 1); + err = write_one_page(page); if (!err) err = sync_inode_metadata(dir, 1); } else { diff --git a/fs/jfs/jfs_metapage.c b/fs/jfs/jfs_metapage.c index 489aaa1403e5..744fa3c079e6 100644 --- a/fs/jfs/jfs_metapage.c +++ b/fs/jfs/jfs_metapage.c @@ -711,7 +711,7 @@ void force_metapage(struct metapage *mp) get_page(page); lock_page(page); set_page_dirty(page); - write_one_page(page, 1); + write_one_page(page); clear_bit(META_forcewrite, &mp->flag); put_page(page); } @@ -756,7 +756,7 @@ void release_metapage(struct metapage * mp) set_page_dirty(page); if (test_bit(META_sync, &mp->flag)) { clear_bit(META_sync, &mp->flag); - write_one_page(page, 1); + write_one_page(page); lock_page(page); /* write_one_page unlocks the page */ } } else if (mp->lsn) /* discard_metapage doesn't remove it */ diff --git a/fs/minix/dir.c b/fs/minix/dir.c index 7edc9b395700..baa9721f1299 100644 --- a/fs/minix/dir.c +++ b/fs/minix/dir.c @@ -57,7 +57,7 @@ static int dir_commit_chunk(struct page *page, loff_t pos, unsigned len) mark_inode_dirty(dir); } if (IS_DIRSYNC(dir)) - err = write_one_page(page, 1); + err = write_one_page(page); else unlock_page(page); return err; diff --git a/fs/sysv/dir.c b/fs/sysv/dir.c index 5bdae85ceef7..f5191cb2c947 100644 --- a/fs/sysv/dir.c +++ b/fs/sysv/dir.c @@ -45,7 +45,7 @@ static int dir_commit_chunk(struct page *page, loff_t pos, unsigned len) mark_inode_dirty(dir); } if (IS_DIRSYNC(dir)) - err = write_one_page(page, 1); + err = write_one_page(page); else unlock_page(page); return err; diff --git a/fs/ufs/dir.c b/fs/ufs/dir.c index de01b8f2aa78..48609f1d9580 100644 --- a/fs/ufs/dir.c +++ b/fs/ufs/dir.c @@ -53,7 +53,7 @@ static int ufs_commit_chunk(struct page *page, loff_t pos, unsigned len) mark_inode_dirty(dir); } if (IS_DIRSYNC(dir)) - err = write_one_page(page, 1); + err = write_one_page(page); else unlock_page(page); return err; diff --git a/include/linux/mm.h b/include/linux/mm.h index 7cb17c6b97de..ca9c8b27cecb 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2199,7 +2199,7 @@ extern void filemap_map_pages(struct vm_fault *vmf, extern int filemap_page_mkwrite(struct vm_fault *vmf); /* mm/page-writeback.c */ -int write_one_page(struct page *page, int wait); +int __must_check write_one_page(struct page *page); void task_dirty_inc(struct task_struct *tsk); /* readahead.c */ diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 143c1c25d680..b901fe52b153 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -2366,15 +2366,16 @@ int do_writepages(struct address_space *mapping, struct writeback_control *wbc) } /** - * write_one_page - write out a single page and optionally wait on I/O + * write_one_page - write out a single page and wait on I/O * @page: the page to write - * @wait: if true, wait on writeout * * The page must be locked by the caller and will be unlocked upon return. * - * write_one_page() returns a negative error code if I/O failed. + * write_one_page() returns a negative error code if I/O failed. Note that + * the address_space is not marked for error. The caller must do this if + * needed. */ -int write_one_page(struct page *page, int wait) +int write_one_page(struct page *page) { struct address_space *mapping = page->mapping; int ret = 0; @@ -2385,13 +2386,12 @@ int write_one_page(struct page *page, int wait) BUG_ON(!PageLocked(page)); - if (wait) - wait_on_page_writeback(page); + wait_on_page_writeback(page); if (clear_page_dirty_for_io(page)) { get_page(page); ret = mapping->a_ops->writepage(page, &wbc); - if (ret == 0 && wait) { + if (ret == 0) { wait_on_page_writeback(page); if (PageError(page)) ret = -EIO; From 11ab83190828cb7b2cbb26467f2eecf74f0214c3 Mon Sep 17 00:00:00 2001 From: Dave Kleikamp Date: Wed, 5 Jul 2017 15:26:49 -0400 Subject: [PATCH 2/4] JFS: do not ignore return code from write_one_page() There are a couple places where jfs calls write_one_page() where clean recovery is not possible. In these cases, the file system should be marked dirty. To do this, it is now necessary to store the superblock in the metapage structure. Link: http://lkml.kernel.org/r/db45ab67-55c7-08ff-6776-f76b3bf5cbf5@oracle.com Signed-off-by: Dave Kleikamp Cc: Jeff Layton Cc: Ross Zwisler Cc: Jan Kara Cc: Matthew Wilcox Cc: Christoph Hellwig Cc: Stephen Rothwell Signed-off-by: Andrew Morton Signed-off-by: Jeff Layton --- fs/jfs/jfs_metapage.c | 7 +++++-- fs/jfs/jfs_metapage.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/fs/jfs/jfs_metapage.c b/fs/jfs/jfs_metapage.c index 744fa3c079e6..7cc1c85f4508 100644 --- a/fs/jfs/jfs_metapage.c +++ b/fs/jfs/jfs_metapage.c @@ -664,6 +664,7 @@ struct metapage *__get_metapage(struct inode *inode, unsigned long lblock, INCREMENT(mpStat.pagealloc); mp = alloc_metapage(GFP_NOFS); mp->page = page; + mp->sb = inode->i_sb; mp->flag = 0; mp->xflag = COMMIT_PAGE; mp->count = 1; @@ -711,7 +712,8 @@ void force_metapage(struct metapage *mp) get_page(page); lock_page(page); set_page_dirty(page); - write_one_page(page); + if (write_one_page(page)) + jfs_error(mp->sb, "write_one_page() failed\n"); clear_bit(META_forcewrite, &mp->flag); put_page(page); } @@ -756,7 +758,8 @@ void release_metapage(struct metapage * mp) set_page_dirty(page); if (test_bit(META_sync, &mp->flag)) { clear_bit(META_sync, &mp->flag); - write_one_page(page); + if (write_one_page(page)) + jfs_error(mp->sb, "write_one_page() failed\n"); lock_page(page); /* write_one_page unlocks the page */ } } else if (mp->lsn) /* discard_metapage doesn't remove it */ diff --git a/fs/jfs/jfs_metapage.h b/fs/jfs/jfs_metapage.h index a869fb4a20d6..8b0ee514eb84 100644 --- a/fs/jfs/jfs_metapage.h +++ b/fs/jfs/jfs_metapage.h @@ -38,6 +38,7 @@ struct metapage { /* implementation */ struct page *page; + struct super_block *sb; unsigned int logical_size; /* Journal management */ From 37e51a7640c275999ea0c35410c42e6d896ff7fa Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 5 Jul 2017 15:17:19 -0400 Subject: [PATCH 3/4] mm: clean up error handling in write_one_page Don't try to check PageError since that's potentially racy and not necessarily going to be set after writepage errors out. Instead, check the mapping for an error after writepage returns. That should also help us detect errors that occurred if the VM tried to clean the page earlier due to memory pressure. Signed-off-by: Jeff Layton Reviewed-by: Jan Kara --- mm/page-writeback.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/mm/page-writeback.c b/mm/page-writeback.c index b901fe52b153..db30ce0b7d80 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -2371,9 +2371,8 @@ int do_writepages(struct address_space *mapping, struct writeback_control *wbc) * * The page must be locked by the caller and will be unlocked upon return. * - * write_one_page() returns a negative error code if I/O failed. Note that - * the address_space is not marked for error. The caller must do this if - * needed. + * Note that the mapping's AS_EIO/AS_ENOSPC flags will be cleared when this + * function returns. */ int write_one_page(struct page *page) { @@ -2391,15 +2390,15 @@ int write_one_page(struct page *page) if (clear_page_dirty_for_io(page)) { get_page(page); ret = mapping->a_ops->writepage(page, &wbc); - if (ret == 0) { + if (ret == 0) wait_on_page_writeback(page); - if (PageError(page)) - ret = -EIO; - } put_page(page); } else { unlock_page(page); } + + if (!ret) + ret = filemap_check_errors(mapping); return ret; } EXPORT_SYMBOL(write_one_page); From 0f41074a65757b46acbdd4293f0de8a70b147406 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 5 Jul 2017 15:26:50 -0400 Subject: [PATCH 4/4] fs: remove call_fsync helper function Reviewed-by: Christoph Hellwig Reviewed-by: Jan Kara Reviewed-by: Carlos Maiolino Signed-off-by: Jeff Layton --- fs/sync.c | 2 +- include/linux/fs.h | 6 ------ ipc/shm.c | 2 +- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/fs/sync.c b/fs/sync.c index 11ba023434b1..2a54c1f22035 100644 --- a/fs/sync.c +++ b/fs/sync.c @@ -192,7 +192,7 @@ int vfs_fsync_range(struct file *file, loff_t start, loff_t end, int datasync) spin_unlock(&inode->i_lock); mark_inode_dirty_sync(inode); } - return call_fsync(file, start, end, datasync); + return file->f_op->fsync(file, start, end, datasync); } EXPORT_SYMBOL(vfs_fsync_range); diff --git a/include/linux/fs.h b/include/linux/fs.h index 803e5a9b2654..2d9e71e2a308 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1739,12 +1739,6 @@ static inline int call_mmap(struct file *file, struct vm_area_struct *vma) return file->f_op->mmap(file, vma); } -static inline int call_fsync(struct file *file, loff_t start, loff_t end, - int datasync) -{ - return file->f_op->fsync(file, start, end, datasync); -} - ssize_t rw_copy_check_uvector(int type, const struct iovec __user * uvector, unsigned long nr_segs, unsigned long fast_segs, struct iovec *fast_pointer, diff --git a/ipc/shm.c b/ipc/shm.c index 34c4344e8d4b..f45c7959b264 100644 --- a/ipc/shm.c +++ b/ipc/shm.c @@ -452,7 +452,7 @@ static int shm_fsync(struct file *file, loff_t start, loff_t end, int datasync) if (!sfd->file->f_op->fsync) return -EINVAL; - return call_fsync(sfd->file, start, end, datasync); + return sfd->file->f_op->fsync(sfd->file, start, end, datasync); } static long shm_fallocate(struct file *file, int mode, loff_t offset,