Re: [PATCH-v2 1/2] fs: make sure the timestamps for lazytime inodes eventually get written

[Jan Kara <jack@suse.cz>]

On Mon 16-03-15 15:14:19, Ted Tso wrote:
> Jan Kara pointed out that if there is an inode which is constantly
> getting dirtied with I_DIRTY_PAGES, an inode with an updated timestamp
> will never be written since inode->dirtied_when is constantly getting
> updated.  We fix this by adding an extra field to the inode,
> dirtied_time_when, so inodes with a stale dirtytime can get detected
> and handled.
> 
> In addition, if we have a dirtytime inode caused by an atime update,
> and there is no write activity on the file system, we need to have a
> secondary system to make sure these inodes get written out.  We do
> this by setting up a second delayed work structure which wakes up the
> CPU much more rarely compared to writeback_expire_centisecs.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Cc: stable@vger.kernel.org
  Since this was merged in rc1, I don't think CC to stable is needed.

> @@ -505,12 +518,17 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
>  	spin_lock(&inode->i_lock);
>  
>  	dirty = inode->i_state & I_DIRTY;
> -	if (((dirty & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) &&
> -	     (inode->i_state & I_DIRTY_TIME)) ||
> -	    (inode->i_state & I_DIRTY_TIME_EXPIRED)) {
> -		dirty |= I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED;
> -		trace_writeback_lazytime(inode);
> -	}
> +	if (inode->i_state & I_DIRTY_TIME) {
> +		if ((dirty & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) ||
> +		    unlikely(inode->i_state & I_DIRTY_TIME_EXPIRED) ||
> +		    unlikely(time_after((inode->dirtied_time_when +
> +					 dirtytime_expire_interval * HZ),
> +					jiffies))) {
  The time comparison is the other way around, isn't it? After fixing that
feel free to add:
  Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

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