linux-ext4 - Re: [RFC PATCH 2/2] mm: Gate stable page writes on the bdi flag

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20121031085812.GD19591@blackbox.djwong.org>
Date:	Wed, 31 Oct 2012 01:58:12 -0700
From:	"Darrick J. Wong" <darrick.wong@...cle.com>
To:	Jan Kara <jack@...e.cz>
Cc:	"Theodore Ts'o" <tytso@....edu>,
	linux-ext4 <linux-ext4@...r.kernel.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [RFC PATCH 2/2] mm: Gate stable page writes on the bdi flag

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@...cle.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@...e.cz>
> SUSE Labs, CR
> --
> 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
--
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