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.w...@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 <swhit...@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 majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to