From mboxrd@z Thu Jan  1 00:00:00 1970
From: Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH] writeback:  Update dirty flags in two steps
Date: Fri, 7 May 2010 08:23:27 -0400
Message-ID: <20100507122327.GA27458@infradead.org>
References: <877hngf40f.fsf@openvz.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: linux-fsdevel@vger.kernel.org, jens.axboe@oracle.com,
	chris.mason@oracle.com
To: Dmitry Monakhov <dmonakhov@openvz.org>
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from bombadil.infradead.org ([18.85.46.34]:58828 "EHLO
	bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753670Ab0EGMX2 (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Fri, 7 May 2010 08:23:28 -0400
Content-Disposition: inline
In-Reply-To: <877hngf40f.fsf@openvz.org>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Fri, May 07, 2010 at 01:35:44PM +0400, Dmitry Monakhov wrote:
> 
> Filesystems with delalloc support may dirty inode during writepages.
> As result inode will have dirty metadata flags even after write_inode.
> In fact we have two dedicated functions for proper data and metadata
> writeback. It is reasonable to separate flags updates in two stages.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=15906
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>

Looks good and passes XFSQA for me.


Reviewed-by: Christoph Hellwig <hch@lst.de>

> +	spin_lock(&inode_lock);
> +	dirty = inode->i_state & I_DIRTY;
> +	inode->i_state &= ~(I_DIRTY_SYNC | I_DIRTY_DATASYNC);
> +	spin_unlock(&inode_lock);
>  	/* Don't write the inode if only I_DIRTY_PAGES was set */
>  	if (dirty & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) {

No need for the double masking, the first line could be just

	dirty = inode->i_state;

or you could do:

	/* Don't write the inode if only I_DIRTY_PAGES was set */
	dirty = inode->i_state & (I_DIRTY_SYNC | I_DIRTY_DATASYNC);

and then just

	if (dirty) {