Re: [PATCH V6] ceph: use vfs __set_page_dirty_nobuffers interface instead of doing it inside filesystem

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 21 Aug 2013, Sha Zhengju wrote:
> Following we will begin to add memcg dirty page accounting around
> __set_page_dirty_{buffers,nobuffers} in vfs layer, so we'd better use vfs interface to
> avoid exporting those details to filesystems.
> 
> Since vfs set_page_dirty() should be called under page lock, here we don't need elaborate
> codes to handle racy anymore, and two WARN_ON() are added to detect such exceptions.
> Thanks very much for Sage and Yan Zheng's coaching!
> 
> I tested it in a two server's ceph environment that one is client and the other is
> mds/osd/mon, and run the following fsx test from xfstests:
> 
>   ./fsx   1MB -N 50000 -p 10000 -l 1048576
>   ./fsx  10MB -N 50000 -p 10000 -l 10485760
>   ./fsx 100MB -N 50000 -p 10000 -l 104857600
> 
> The fsx does lots of mmap-read/mmap-write/truncate operations and the tests completed
> successfully without triggering any of WARN_ON.
> 
> Signed-off-by: Sha Zhengju <handai.szj@xxxxxxxxxx>

Reviewed-by: Sage Weil <sage@xxxxxxxxxxx>

Would you like me to take this through the ceph tree?

Thanks-
sage


> ---
>  fs/ceph/addr.c |   42 ++++++++++++++----------------------------
>  1 file changed, 14 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index afb2fc2..01891f4 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -72,13 +72,15 @@ static int ceph_set_page_dirty(struct page *page)
>  	struct ceph_inode_info *ci;
>  	int undo = 0;
>  	struct ceph_snap_context *snapc;
> +	int ret;
>  
>  	if (unlikely(!mapping))
>  		return !TestSetPageDirty(page);
>  
> -	if (TestSetPageDirty(page)) {
> +	if (PageDirty(page)) {
>  		dout("%p set_page_dirty %p idx %lu -- already dirty\n",
>  		     mapping->host, page, page->index);
> +		BUG_ON(!PagePrivate(page));
>  		return 0;
>  	}
>  
> @@ -107,35 +109,19 @@ static int ceph_set_page_dirty(struct page *page)
>  	     snapc, snapc->seq, snapc->num_snaps);
>  	spin_unlock(&ci->i_ceph_lock);
>  
> -	/* now adjust page */
> -	spin_lock_irq(&mapping->tree_lock);
> -	if (page->mapping) {	/* Race with truncate? */
> -		WARN_ON_ONCE(!PageUptodate(page));
> -		account_page_dirtied(page, page->mapping);
> -		radix_tree_tag_set(&mapping->page_tree,
> -				page_index(page), PAGECACHE_TAG_DIRTY);
> -
> -		/*
> -		 * Reference snap context in page->private.  Also set
> -		 * PagePrivate so that we get invalidatepage callback.
> -		 */
> -		page->private = (unsigned long)snapc;
> -		SetPagePrivate(page);
> -	} else {
> -		dout("ANON set_page_dirty %p (raced truncate?)\n", page);
> -		undo = 1;
> -	}
> -
> -	spin_unlock_irq(&mapping->tree_lock);
> -
> -	if (undo)
> -		/* whoops, we failed to dirty the page */
> -		ceph_put_wrbuffer_cap_refs(ci, 1, snapc);
> +	/*
> +	 * Reference snap context in page->private.  Also set
> +	 * PagePrivate so that we get invalidatepage callback.
> +	 */
> +	BUG_ON(PagePrivate(page));
> +	page->private = (unsigned long)snapc;
> +	SetPagePrivate(page);
>  
> -	__mark_inode_dirty(mapping->host, I_DIRTY_PAGES);
> +	ret = __set_page_dirty_nobuffers(page);
> +	WARN_ON(!PageLocked(page));
> +	WARN_ON(!page->mapping);
>  
> -	BUG_ON(!PageDirty(page));
> -	return 1;
> +	return ret;
>  }
>  
>  /*
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux