Re: [PATCH] writeback: Update dirty flags in two steps

[Christoph Hellwig <hch@infradead.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) {