| Message ID | 20130115054250.1563.40740.stgit@blackbox.djwong.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Mon 14-01-13 21:42:50, Darrick J. Wong wrote: > Create a helper function to check if a backing device requires stable page > writes and, if so, performs the necessary wait. Then, make it so that all > points in the memory manager that handle making pages writable use the helper > function. This should provide stable page write support to most filesystems, > while eliminating unnecessary waiting for devices that don't require the > feature. Looks good. You can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/buffer.c | 2 +- > fs/ext4/inode.c | 2 +- > fs/gfs2/file.c | 2 +- > fs/nilfs2/file.c | 2 +- > include/linux/pagemap.h | 1 + > mm/filemap.c | 3 ++- > mm/page-writeback.c | 20 ++++++++++++++++++++ > 7 files changed, 27 insertions(+), 5 deletions(-) > > > diff --git a/fs/buffer.c b/fs/buffer.c > index c017a2d..2981449 100644 > --- a/fs/buffer.c > +++ b/fs/buffer.c > @@ -2359,7 +2359,7 @@ int __block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf, > if (unlikely(ret < 0)) > goto out_unlock; > set_page_dirty(page); > - wait_on_page_writeback(page); > + wait_for_stable_page(page); > return 0; > out_unlock: > unlock_page(page); > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index cbfe13b..cd818d8b 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -4968,7 +4968,7 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) > 0, len, NULL, > ext4_bh_unmapped)) { > /* Wait so that we don't change page under IO */ > - wait_on_page_writeback(page); > + wait_for_stable_page(page); > ret = VM_FAULT_LOCKED; > goto out; > } > diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c > index 991ab2d..b9e0ca2 100644 > --- a/fs/gfs2/file.c > +++ b/fs/gfs2/file.c > @@ -483,7 +483,7 @@ out: > gfs2_holder_uninit(&gh); > if (ret == 0) { > set_page_dirty(page); > - wait_on_page_writeback(page); > + wait_for_stable_page(page); > } > sb_end_pagefault(inode->i_sb); > return block_page_mkwrite_return(ret); > diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c > index 6194688..bec4af6 100644 > --- a/fs/nilfs2/file.c > +++ b/fs/nilfs2/file.c > @@ -126,7 +126,7 @@ static int nilfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) > nilfs_transaction_commit(inode->i_sb); > > mapped: > - wait_on_page_writeback(page); > + wait_for_stable_page(page); > out: > sb_end_pagefault(inode->i_sb); > return block_page_mkwrite_return(ret); > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > index 6da609d..0e38e13 100644 > --- a/include/linux/pagemap.h > +++ b/include/linux/pagemap.h > @@ -414,6 +414,7 @@ static inline void wait_on_page_writeback(struct page *page) > } > > extern void end_page_writeback(struct page *page); > +void wait_for_stable_page(struct page *page); > > /* > * Add an arbitrary waiter to a page's wait queue > diff --git a/mm/filemap.c b/mm/filemap.c > index 83efee7..5577dc8 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1728,6 +1728,7 @@ int filemap_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) > * see the dirty page and writeprotect it again. > */ > set_page_dirty(page); > + wait_for_stable_page(page); > out: > sb_end_pagefault(inode->i_sb); > return ret; > @@ -2274,7 +2275,7 @@ repeat: > return NULL; > } > found: > - wait_on_page_writeback(page); > + wait_for_stable_page(page); > return page; > } > EXPORT_SYMBOL(grab_cache_page_write_begin); > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index 0713bfb..9c5af4d 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -2289,3 +2289,23 @@ int mapping_tagged(struct address_space *mapping, int tag) > return radix_tree_tagged(&mapping->page_tree, tag); > } > EXPORT_SYMBOL(mapping_tagged); > + > +/** > + * wait_for_stable_page() - wait for writeback to finish, if necessary. > + * @page: The page to wait on. > + * > + * This function determines if the given page is related to a backing device > + * that requires page contents to be held stable during writeback. If so, then > + * it will wait for any pending writeback to complete. > + */ > +void wait_for_stable_page(struct page *page) > +{ > + struct address_space *mapping = page_mapping(page); > + struct backing_dev_info *bdi = mapping->backing_dev_info; > + > + if (!bdi_cap_stable_pages_required(bdi)) > + return; > + > + wait_on_page_writeback(page); > +} > +EXPORT_SYMBOL_GPL(wait_for_stable_page); >
Hi, On Mon, 2013-01-14 at 21:42 -0800, Darrick J. Wong wrote: > Create a helper function to check if a backing device requires stable page > writes and, if so, performs the necessary wait. Then, make it so that all > points in the memory manager that handle making pages writable use the helper > function. This should provide stable page write support to most filesystems, > while eliminating unnecessary waiting for devices that don't require the > feature. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/buffer.c | 2 +- > fs/ext4/inode.c | 2 +- > fs/gfs2/file.c | 2 +- > fs/nilfs2/file.c | 2 +- > include/linux/pagemap.h | 1 + > mm/filemap.c | 3 ++- > mm/page-writeback.c | 20 ++++++++++++++++++++ > 7 files changed, 27 insertions(+), 5 deletions(-) > Acked-by: Steven Whitehouse <swhiteho@redhat.com> Are there any other places that need patching in order to have stable page writes working with GFS2? Do we need to unmap in certain places or are the hooks for that already in the core code? Steve. > > diff --git a/fs/buffer.c b/fs/buffer.c > index c017a2d..2981449 100644 > --- a/fs/buffer.c > +++ b/fs/buffer.c > @@ -2359,7 +2359,7 @@ int __block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf, > if (unlikely(ret < 0)) > goto out_unlock; > set_page_dirty(page); > - wait_on_page_writeback(page); > + wait_for_stable_page(page); > return 0; > out_unlock: > unlock_page(page); > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index cbfe13b..cd818d8b 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -4968,7 +4968,7 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) > 0, len, NULL, > ext4_bh_unmapped)) { > /* Wait so that we don't change page under IO */ > - wait_on_page_writeback(page); > + wait_for_stable_page(page); > ret = VM_FAULT_LOCKED; > goto out; > } > diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c > index 991ab2d..b9e0ca2 100644 > --- a/fs/gfs2/file.c > +++ b/fs/gfs2/file.c > @@ -483,7 +483,7 @@ out: > gfs2_holder_uninit(&gh); > if (ret == 0) { > set_page_dirty(page); > - wait_on_page_writeback(page); > + wait_for_stable_page(page); > } > sb_end_pagefault(inode->i_sb); > return block_page_mkwrite_return(ret); > diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c > index 6194688..bec4af6 100644 > --- a/fs/nilfs2/file.c > +++ b/fs/nilfs2/file.c > @@ -126,7 +126,7 @@ static int nilfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) > nilfs_transaction_commit(inode->i_sb); > > mapped: > - wait_on_page_writeback(page); > + wait_for_stable_page(page); > out: > sb_end_pagefault(inode->i_sb); > return block_page_mkwrite_return(ret); > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > index 6da609d..0e38e13 100644 > --- a/include/linux/pagemap.h > +++ b/include/linux/pagemap.h > @@ -414,6 +414,7 @@ static inline void wait_on_page_writeback(struct page *page) > } > > extern void end_page_writeback(struct page *page); > +void wait_for_stable_page(struct page *page); > > /* > * Add an arbitrary waiter to a page's wait queue > diff --git a/mm/filemap.c b/mm/filemap.c > index 83efee7..5577dc8 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1728,6 +1728,7 @@ int filemap_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) > * see the dirty page and writeprotect it again. > */ > set_page_dirty(page); > + wait_for_stable_page(page); > out: > sb_end_pagefault(inode->i_sb); > return ret; > @@ -2274,7 +2275,7 @@ repeat: > return NULL; > } > found: > - wait_on_page_writeback(page); > + wait_for_stable_page(page); > return page; > } > EXPORT_SYMBOL(grab_cache_page_write_begin); > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index 0713bfb..9c5af4d 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -2289,3 +2289,23 @@ int mapping_tagged(struct address_space *mapping, int tag) > return radix_tree_tagged(&mapping->page_tree, tag); > } > EXPORT_SYMBOL(mapping_tagged); > + > +/** > + * wait_for_stable_page() - wait for writeback to finish, if necessary. > + * @page: The page to wait on. > + * > + * This function determines if the given page is related to a backing device > + * that requires page contents to be held stable during writeback. If so, then > + * it will wait for any pending writeback to complete. > + */ > +void wait_for_stable_page(struct page *page) > +{ > + struct address_space *mapping = page_mapping(page); > + struct backing_dev_info *bdi = mapping->backing_dev_info; > + > + if (!bdi_cap_stable_pages_required(bdi)) > + return; > + > + wait_on_page_writeback(page); > +} > +EXPORT_SYMBOL_GPL(wait_for_stable_page); > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 15, 2013 at 10:59:26AM +0000, Steven Whitehouse wrote: > Hi, > > On Mon, 2013-01-14 at 21:42 -0800, Darrick J. Wong wrote: > > Create a helper function to check if a backing device requires stable page > > writes and, if so, performs the necessary wait. Then, make it so that all > > points in the memory manager that handle making pages writable use the helper > > function. This should provide stable page write support to most filesystems, > > while eliminating unnecessary waiting for devices that don't require the > > feature. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > fs/buffer.c | 2 +- > > fs/ext4/inode.c | 2 +- > > fs/gfs2/file.c | 2 +- > > fs/nilfs2/file.c | 2 +- > > include/linux/pagemap.h | 1 + > > mm/filemap.c | 3 ++- > > mm/page-writeback.c | 20 ++++++++++++++++++++ > > 7 files changed, 27 insertions(+), 5 deletions(-) > > > Acked-by: Steven Whitehouse <swhiteho@redhat.com> > > Are there any other places that need patching in order to have stable > page writes working with GFS2? Do we need to unmap in certain places or > are the hooks for that already in the core code? From my semi-brief readthrough of gfs2, I think it's fine, though if you want to test, you could create a "DIF" checksumming disk via scsi-debug: # modprobe scsi-debug dif=1 dix=199 ato=1 [more params...] and create a gfs2 instance atop that. One other thing to look for is writing metadata objects to buffer_heads or pages without locking them first. If the fs is dirtying metadata concurrent with it being written out to disk, that'll cause problems. Thanks for testing! --D > > Steve. > > > > > diff --git a/fs/buffer.c b/fs/buffer.c > > index c017a2d..2981449 100644 > > --- a/fs/buffer.c > > +++ b/fs/buffer.c > > @@ -2359,7 +2359,7 @@ int __block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf, > > if (unlikely(ret < 0)) > > goto out_unlock; > > set_page_dirty(page); > > - wait_on_page_writeback(page); > > + wait_for_stable_page(page); > > return 0; > > out_unlock: > > unlock_page(page); > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > > index cbfe13b..cd818d8b 100644 > > --- a/fs/ext4/inode.c > > +++ b/fs/ext4/inode.c > > @@ -4968,7 +4968,7 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) > > 0, len, NULL, > > ext4_bh_unmapped)) { > > /* Wait so that we don't change page under IO */ > > - wait_on_page_writeback(page); > > + wait_for_stable_page(page); > > ret = VM_FAULT_LOCKED; > > goto out; > > } > > diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c > > index 991ab2d..b9e0ca2 100644 > > --- a/fs/gfs2/file.c > > +++ b/fs/gfs2/file.c > > @@ -483,7 +483,7 @@ out: > > gfs2_holder_uninit(&gh); > > if (ret == 0) { > > set_page_dirty(page); > > - wait_on_page_writeback(page); > > + wait_for_stable_page(page); > > } > > sb_end_pagefault(inode->i_sb); > > return block_page_mkwrite_return(ret); > > diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c > > index 6194688..bec4af6 100644 > > --- a/fs/nilfs2/file.c > > +++ b/fs/nilfs2/file.c > > @@ -126,7 +126,7 @@ static int nilfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) > > nilfs_transaction_commit(inode->i_sb); > > > > mapped: > > - wait_on_page_writeback(page); > > + wait_for_stable_page(page); > > out: > > sb_end_pagefault(inode->i_sb); > > return block_page_mkwrite_return(ret); > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > > index 6da609d..0e38e13 100644 > > --- a/include/linux/pagemap.h > > +++ b/include/linux/pagemap.h > > @@ -414,6 +414,7 @@ static inline void wait_on_page_writeback(struct page *page) > > } > > > > extern void end_page_writeback(struct page *page); > > +void wait_for_stable_page(struct page *page); > > > > /* > > * Add an arbitrary waiter to a page's wait queue > > diff --git a/mm/filemap.c b/mm/filemap.c > > index 83efee7..5577dc8 100644 > > --- a/mm/filemap.c > > +++ b/mm/filemap.c > > @@ -1728,6 +1728,7 @@ int filemap_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) > > * see the dirty page and writeprotect it again. > > */ > > set_page_dirty(page); > > + wait_for_stable_page(page); > > out: > > sb_end_pagefault(inode->i_sb); > > return ret; > > @@ -2274,7 +2275,7 @@ repeat: > > return NULL; > > } > > found: > > - wait_on_page_writeback(page); > > + wait_for_stable_page(page); > > return page; > > } > > EXPORT_SYMBOL(grab_cache_page_write_begin); > > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > > index 0713bfb..9c5af4d 100644 > > --- a/mm/page-writeback.c > > +++ b/mm/page-writeback.c > > @@ -2289,3 +2289,23 @@ int mapping_tagged(struct address_space *mapping, int tag) > > return radix_tree_tagged(&mapping->page_tree, tag); > > } > > EXPORT_SYMBOL(mapping_tagged); > > + > > +/** > > + * wait_for_stable_page() - wait for writeback to finish, if necessary. > > + * @page: The page to wait on. > > + * > > + * This function determines if the given page is related to a backing device > > + * that requires page contents to be held stable during writeback. If so, then > > + * it will wait for any pending writeback to complete. > > + */ > > +void wait_for_stable_page(struct page *page) > > +{ > > + struct address_space *mapping = page_mapping(page); > > + struct backing_dev_info *bdi = mapping->backing_dev_info; > > + > > + if (!bdi_cap_stable_pages_required(bdi)) > > + return; > > + > > + wait_on_page_writeback(page); > > +} > > +EXPORT_SYMBOL_GPL(wait_for_stable_page); > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/buffer.c b/fs/buffer.c index c017a2d..2981449 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -2359,7 +2359,7 @@ int __block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf, if (unlikely(ret < 0)) goto out_unlock; set_page_dirty(page); - wait_on_page_writeback(page); + wait_for_stable_page(page); return 0; out_unlock: unlock_page(page); diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index cbfe13b..cd818d8b 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -4968,7 +4968,7 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) 0, len, NULL, ext4_bh_unmapped)) { /* Wait so that we don't change page under IO */ - wait_on_page_writeback(page); + wait_for_stable_page(page); ret = VM_FAULT_LOCKED; goto out; } diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c index 991ab2d..b9e0ca2 100644 --- a/fs/gfs2/file.c +++ b/fs/gfs2/file.c @@ -483,7 +483,7 @@ out: gfs2_holder_uninit(&gh); if (ret == 0) { set_page_dirty(page); - wait_on_page_writeback(page); + wait_for_stable_page(page); } sb_end_pagefault(inode->i_sb); return block_page_mkwrite_return(ret); diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c index 6194688..bec4af6 100644 --- a/fs/nilfs2/file.c +++ b/fs/nilfs2/file.c @@ -126,7 +126,7 @@ static int nilfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) nilfs_transaction_commit(inode->i_sb); mapped: - wait_on_page_writeback(page); + wait_for_stable_page(page); out: sb_end_pagefault(inode->i_sb); return block_page_mkwrite_return(ret); diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index 6da609d..0e38e13 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -414,6 +414,7 @@ static inline void wait_on_page_writeback(struct page *page) } extern void end_page_writeback(struct page *page); +void wait_for_stable_page(struct page *page); /* * Add an arbitrary waiter to a page's wait queue diff --git a/mm/filemap.c b/mm/filemap.c index 83efee7..5577dc8 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1728,6 +1728,7 @@ int filemap_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) * see the dirty page and writeprotect it again. */ set_page_dirty(page); + wait_for_stable_page(page); out: sb_end_pagefault(inode->i_sb); return ret; @@ -2274,7 +2275,7 @@ repeat: return NULL; } found: - wait_on_page_writeback(page); + wait_for_stable_page(page); return page; } EXPORT_SYMBOL(grab_cache_page_write_begin); diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 0713bfb..9c5af4d 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -2289,3 +2289,23 @@ int mapping_tagged(struct address_space *mapping, int tag) return radix_tree_tagged(&mapping->page_tree, tag); } EXPORT_SYMBOL(mapping_tagged); + +/** + * wait_for_stable_page() - wait for writeback to finish, if necessary. + * @page: The page to wait on. + * + * This function determines if the given page is related to a backing device + * that requires page contents to be held stable during writeback. If so, then + * it will wait for any pending writeback to complete. + */ +void wait_for_stable_page(struct page *page) +{ + struct address_space *mapping = page_mapping(page); + struct backing_dev_info *bdi = mapping->backing_dev_info; + + if (!bdi_cap_stable_pages_required(bdi)) + return; + + wait_on_page_writeback(page); +} +EXPORT_SYMBOL_GPL(wait_for_stable_page);
Create a helper function to check if a backing device requires stable page writes and, if so, performs the necessary wait. Then, make it so that all points in the memory manager that handle making pages writable use the helper function. This should provide stable page write support to most filesystems, while eliminating unnecessary waiting for devices that don't require the feature. Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- fs/buffer.c | 2 +- fs/ext4/inode.c | 2 +- fs/gfs2/file.c | 2 +- fs/nilfs2/file.c | 2 +- include/linux/pagemap.h | 1 + mm/filemap.c | 3 ++- mm/page-writeback.c | 20 ++++++++++++++++++++ 7 files changed, 27 insertions(+), 5 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html