Re: [PATCH] fs: clear I_DIRTY_TIME in sync_lazytime

[Jan Kara <jack@suse.cz>]

On Tue 17-03-26 14:44:04, Christoph Hellwig wrote:
> For file systems implementing ->sync_lazytime, I_DIRTY_TIME fails to get
> cleared in sync_lazytime, and might cause additional calls to
> sync_lazytime during inode deactivation.  Use the same pattern as in
> __mark_inode_dirty to clear the flag under the inode lock.
> 
> Fixes: 5cf06ea56ee6 ("fs: add a ->sync_lazytime method")
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/fs-writeback.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 7c75ed7e8979..d8dac1931595 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1711,6 +1711,19 @@ static void requeue_inode(struct inode *inode, struct bdi_writeback *wb,
>  	}
>  }
>  
> +static bool __sync_lazytime(struct inode *inode)
> +{
> +	spin_lock(&inode->i_lock);
> +	if (!(inode_state_read(inode) & I_DIRTY_TIME)) {
> +		spin_unlock(&inode->i_lock);
> +		return false;
> +	}
> +	inode_state_clear(inode, I_DIRTY_TIME);
> +	spin_unlock(&inode->i_lock);
> +	inode->i_op->sync_lazytime(inode);
> +	return true;
> +}
> +
>  bool sync_lazytime(struct inode *inode)
>  {
>  	if (!(inode_state_read_once(inode) & I_DIRTY_TIME))
> @@ -1718,9 +1731,8 @@ bool sync_lazytime(struct inode *inode)
>  
>  	trace_writeback_lazytime(inode);
>  	if (inode->i_op->sync_lazytime)
> -		inode->i_op->sync_lazytime(inode);
> -	else
> -		mark_inode_dirty_sync(inode);
> +		return __sync_lazytime(inode);
> +	mark_inode_dirty_sync(inode);
>  	return true;
>  }
>  
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR