lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Thu, 17 Jan 2013 17:26:36 -0800
From:	"Darrick J. Wong" <darrick.wong@...cle.com>
To:	Steven Whitehouse <swhiteho@...hat.com>
Cc:	axboe@...nel.dk, lucho@...kov.net, jack@...e.cz, ericvh@...il.com,
	tytso@....edu, viro@...iv.linux.org.uk, rminnich@...dia.gov,
	martin.petersen@...cle.com, neilb@...e.de, david@...morbit.com,
	gnehzuil.liu@...il.com, linux-kernel@...r.kernel.org,
	hch@...radead.org, linux-fsdevel@...r.kernel.org,
	adilger.kernel@...ger.ca, bharrosh@...asas.com, jlayton@...ba.org,
	linux-ext4@...r.kernel.org, hirofumi@...l.parknet.co.jp
Subject: Re: [PATCH 2/6] mm: Only enforce stable page writes if the backing
 device requires it

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@...cle.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@...hat.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@...r.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists