Re: [PATCH 6/6] dax: Avoid page invalidation races and unnecessary radix tree traversals

[Jan Kara <jack@suse.cz>]

On Fri 30-09-16 01:55:44, Christoph Hellwig wrote:
> On Tue, Sep 27, 2016 at 06:43:35PM +0200, Jan Kara wrote:
> > Currently each filesystem (possibly through generic_file_direct_write()
> > or iomap_dax_rw()) takes care of invalidating page tables and evicting
> > hole pages from the radix tree when write(2) to the file happens. This
> > invalidation is only necessary when there is some block allocation
> > resulting from write(2). Furthermore in current place the invalidation
> > is racy wrt page fault instantiating a hole page just after we have
> > invalidated it.
> > 
> > So perform the page invalidation inside dax_do_io() where we can do it
> > only when really necessary and after blocks have been allocated so
> > nobody will be instantiating new hole pages anymore.
> > 
> > Signed-off-by: Jan Kara <jack@suse.cz>
> 
> This looks fine with the comment from Dave addressed:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks.

> > +				if (buffer_new(bh) &&
> > +				    inode->i_mapping->nrpages) {
> 
> Btw, it would be nice if the nrpages check could move into
> invalidate_inode_pages2_range instead of having to bother with it in
> the callers.

We cannot do that - someone can be possibly calling
invalidate_inode_pages2_range() to invalidate exceptional entries in the
given range (although I don't think there's currently any such caller). DAX
code currently needs it only to invalidate hole pages so that's why it can
do an optimization to call invalidate_inode_pages2_range() only when
mapping->nrpages > 0. But in general it would be a trap for
invalidate_inode_pages2_range() to work only if mapping->nrpages > 0...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR