linux-ext4 - Re: [PATCH 3/4] vfs: don't let the dirty time inodes get more than a day stale

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141121213651.GF7112@thunk.org>
Date:	Fri, 21 Nov 2014 16:36:51 -0500
From:	Theodore Ts'o <tytso@....edu>
To:	Andreas Dilger <adilger@...ger.ca>
Cc:	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	Ext4 Developers List <linux-ext4@...r.kernel.org>,
	xfs@....sgi.com, linux-btrfs@...r.kernel.org
Subject: Re: [PATCH 3/4] vfs: don't let the dirty time inodes get more than a
 day stale

On Fri, Nov 21, 2014 at 02:19:07PM -0600, Andreas Dilger wrote:
> > -	if (inode->i_sb->s_flags & MS_LAZYTIME) {
> > +	if ((inode->i_sb->s_flags & MS_LAZYTIME) &&
> > +	    (!inode->i_ts_dirty_day ||
> > +	     inode->i_ts_dirty_day == days_since_boot)) {
> > 		spin_lock(&inode->i_lock);
> > 		inode->i_state |= I_DIRTY_TIME;
> > 		spin_unlock(&inode->i_lock);
> > +		inode->i_ts_dirty_day = days_since_boot;
> 
> It isn't clear if this is correct.  It looks like the condition will
> only be entered if i_ts_dirty_day == days_since_boot, but that is only
> set inside the condition?

If i_ts_dirty_day is zero, the timestamps don't have to written to
disk.  This is either because the inode has been written to disk, or
the system has been up for less than a day, such that when we last a
lazy mtime update (i.e., we skipped the call to mark_inode_dirty)
since jiffies / (HZ * 86400) was zero.

If it is non-zero, then the timestamps were updated but were not sent
to disk N days since the system was booted.  So long as it remains N
days since the system was booted, we can skip calling
mark_inode_dirty().  But once it becomes N+1 days since the system was
booted, then we will call mark_inode_dirty() and set i_ts_dirty_day to
zero.

I'll add a comment so it's a bit more obvious what we're doing here,
but I'm pretty sure we currently have is in fact correct.

> and "days_since_boot" should be declared unsigned short so it wraps
> in the same way as i_ts_dirty_day

Good point, thanks.  This will only be an issue after the system has
been up for almost 90 years, but we might as well get it right,

     	    	      	     	    	  - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html