Message ID | 20121027013608.GB19591@blackbox.djwong.org |
---|---|
State | Superseded, archived |
Headers | show |
On Fri 26-10-12 18:36:08, Darrick J. Wong wrote: > Create a helper function to decide if a particular address space is backed by a > device that requires stable page writes. For each wait_on_page_writeback call > that was inserted in the original stable page write patchset, change it to wait > only if the helper says it's necessary. This should eliminate unnecessary > waiting for devices that don't require page contents to be immutable during > writeout. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > > fs/buffer.c | 3 ++- > fs/ext4/inode.c | 4 ++-- > include/linux/fs.h | 2 ++ > mm/filemap.c | 3 ++- > mm/page-writeback.c | 7 +++++++ > 5 files changed, 15 insertions(+), 4 deletions(-) > > diff --git a/fs/buffer.c b/fs/buffer.c > index b5f0442..f508ece 100644 > --- a/fs/buffer.c > +++ b/fs/buffer.c > @@ -2334,7 +2334,8 @@ 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); > + if (page->mapping && mapping_needs_stable_writes(page->mapping)) > + wait_on_page_writeback(page); The page is locked and we checked whether it belongs to the right mapping just after we locked it down. So you can safely skip the page->mapping test. > return 0; > out_unlock: > unlock_page(page); > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index b3c243b..82908b8 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -4813,8 +4813,8 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) > if (page_has_buffers(page)) { > if (!walk_page_buffers(NULL, page_buffers(page), 0, len, NULL, > ext4_bh_unmapped)) { > - /* Wait so that we don't change page under IO */ > - wait_on_page_writeback(page); > + if (mapping_needs_stable_writes(mapping)) > + wait_on_page_writeback(page); > ret = VM_FAULT_LOCKED; > goto out; > } > diff --git a/include/linux/fs.h b/include/linux/fs.h > index b33cfc9..d99db0e 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -466,6 +466,8 @@ struct block_device { > struct percpu_rw_semaphore bd_block_size_semaphore; > }; > > +int mapping_needs_stable_writes(struct address_space *mapping); > + > /* > * Radix-tree tags, for tagging dirty and writeback pages within the pagecache > * radix trees > diff --git a/mm/filemap.c b/mm/filemap.c > index 83efee7..aedc6bd 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -2274,7 +2274,8 @@ repeat: > return NULL; > } > found: > - wait_on_page_writeback(page); > + if (mapping_needs_stable_writes(mapping)) > + wait_on_page_writeback(page); > return page; > } > EXPORT_SYMBOL(grab_cache_page_write_begin); > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index 830893b..d0f042c 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -2275,3 +2275,10 @@ int mapping_tagged(struct address_space *mapping, int tag) > return radix_tree_tagged(&mapping->page_tree, tag); > } > EXPORT_SYMBOL(mapping_tagged); > + > +int mapping_needs_stable_writes(struct address_space *mapping) > +{ > + return mapping->backing_dev_info && > + bdi_cap_stable_writes(mapping->backing_dev_info); > +} > +EXPORT_SYMBOL_GPL(mapping_needs_stable_writes); Traditional name for these shortcuts is "mapping_cap_..." and put them in linux/backing-dev.h. And you don't have to check mapping->backing_dev_info. Honza
On Mon, Oct 29, 2012 at 07:28:57PM +0100, Jan Kara wrote: > On Fri 26-10-12 18:36:08, Darrick J. Wong wrote: > > Create a helper function to decide if a particular address space is backed by a > > device that requires stable page writes. For each wait_on_page_writeback call > > that was inserted in the original stable page write patchset, change it to wait > > only if the helper says it's necessary. This should eliminate unnecessary > > waiting for devices that don't require page contents to be immutable during > > writeout. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > > > fs/buffer.c | 3 ++- > > fs/ext4/inode.c | 4 ++-- > > include/linux/fs.h | 2 ++ > > mm/filemap.c | 3 ++- > > mm/page-writeback.c | 7 +++++++ > > 5 files changed, 15 insertions(+), 4 deletions(-) > > > > diff --git a/fs/buffer.c b/fs/buffer.c > > index b5f0442..f508ece 100644 > > --- a/fs/buffer.c > > +++ b/fs/buffer.c > > @@ -2334,7 +2334,8 @@ 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); > > + if (page->mapping && mapping_needs_stable_writes(page->mapping)) > > + wait_on_page_writeback(page); > The page is locked and we checked whether it belongs to the right mapping > just after we locked it down. So you can safely skip the page->mapping > test. > > > return 0; > > out_unlock: > > unlock_page(page); > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > > index b3c243b..82908b8 100644 > > --- a/fs/ext4/inode.c > > +++ b/fs/ext4/inode.c > > @@ -4813,8 +4813,8 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) > > if (page_has_buffers(page)) { > > if (!walk_page_buffers(NULL, page_buffers(page), 0, len, NULL, > > ext4_bh_unmapped)) { > > - /* Wait so that we don't change page under IO */ > > - wait_on_page_writeback(page); > > + if (mapping_needs_stable_writes(mapping)) > > + wait_on_page_writeback(page); > > ret = VM_FAULT_LOCKED; > > goto out; > > } > > diff --git a/include/linux/fs.h b/include/linux/fs.h > > index b33cfc9..d99db0e 100644 > > --- a/include/linux/fs.h > > +++ b/include/linux/fs.h > > @@ -466,6 +466,8 @@ struct block_device { > > struct percpu_rw_semaphore bd_block_size_semaphore; > > }; > > > > +int mapping_needs_stable_writes(struct address_space *mapping); > > + > > /* > > * Radix-tree tags, for tagging dirty and writeback pages within the pagecache > > * radix trees > > diff --git a/mm/filemap.c b/mm/filemap.c > > index 83efee7..aedc6bd 100644 > > --- a/mm/filemap.c > > +++ b/mm/filemap.c > > @@ -2274,7 +2274,8 @@ repeat: > > return NULL; > > } > > found: > > - wait_on_page_writeback(page); > > + if (mapping_needs_stable_writes(mapping)) > > + wait_on_page_writeback(page); > > return page; > > } > > EXPORT_SYMBOL(grab_cache_page_write_begin); > > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > > index 830893b..d0f042c 100644 > > --- a/mm/page-writeback.c > > +++ b/mm/page-writeback.c > > @@ -2275,3 +2275,10 @@ int mapping_tagged(struct address_space *mapping, int tag) > > return radix_tree_tagged(&mapping->page_tree, tag); > > } > > EXPORT_SYMBOL(mapping_tagged); > > + > > +int mapping_needs_stable_writes(struct address_space *mapping) > > +{ > > + return mapping->backing_dev_info && > > + bdi_cap_stable_writes(mapping->backing_dev_info); > > +} > > +EXPORT_SYMBOL_GPL(mapping_needs_stable_writes); > Traditional name for these shortcuts is "mapping_cap_..." and put them in > linux/backing-dev.h. And you don't have to check > mapping->backing_dev_info. Ok, I've removed both of those checks. Thank you for the comments! --D > > Honza > -- > Jan Kara <jack@suse.cz> > SUSE Labs, CR > -- > 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 -- 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 b5f0442..f508ece 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -2334,7 +2334,8 @@ 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); + if (page->mapping && mapping_needs_stable_writes(page->mapping)) + wait_on_page_writeback(page); return 0; out_unlock: unlock_page(page); diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index b3c243b..82908b8 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -4813,8 +4813,8 @@ int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf) if (page_has_buffers(page)) { if (!walk_page_buffers(NULL, page_buffers(page), 0, len, NULL, ext4_bh_unmapped)) { - /* Wait so that we don't change page under IO */ - wait_on_page_writeback(page); + if (mapping_needs_stable_writes(mapping)) + wait_on_page_writeback(page); ret = VM_FAULT_LOCKED; goto out; } diff --git a/include/linux/fs.h b/include/linux/fs.h index b33cfc9..d99db0e 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -466,6 +466,8 @@ struct block_device { struct percpu_rw_semaphore bd_block_size_semaphore; }; +int mapping_needs_stable_writes(struct address_space *mapping); + /* * Radix-tree tags, for tagging dirty and writeback pages within the pagecache * radix trees diff --git a/mm/filemap.c b/mm/filemap.c index 83efee7..aedc6bd 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -2274,7 +2274,8 @@ repeat: return NULL; } found: - wait_on_page_writeback(page); + if (mapping_needs_stable_writes(mapping)) + wait_on_page_writeback(page); return page; } EXPORT_SYMBOL(grab_cache_page_write_begin); diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 830893b..d0f042c 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -2275,3 +2275,10 @@ int mapping_tagged(struct address_space *mapping, int tag) return radix_tree_tagged(&mapping->page_tree, tag); } EXPORT_SYMBOL(mapping_tagged); + +int mapping_needs_stable_writes(struct address_space *mapping) +{ + return mapping->backing_dev_info && + bdi_cap_stable_writes(mapping->backing_dev_info); +} +EXPORT_SYMBOL_GPL(mapping_needs_stable_writes);
Create a helper function to decide if a particular address space is backed by a device that requires stable page writes. For each wait_on_page_writeback call that was inserted in the original stable page write patchset, change it to wait only if the helper says it's necessary. This should eliminate unnecessary waiting for devices that don't require page contents to be immutable during writeout. Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- fs/buffer.c | 3 ++- fs/ext4/inode.c | 4 ++-- include/linux/fs.h | 2 ++ mm/filemap.c | 3 ++- mm/page-writeback.c | 7 +++++++ 5 files changed, 15 insertions(+), 4 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