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

["Theodore Ts'o" <tytso@mit.edu>]

On Mon, Nov 24, 2014 at 01:27:21PM +0100, Rasmus Villemoes wrote:
> On Sat, Nov 22 2014, Theodore Ts'o <tytso@mit.edu> wrote:
> 
> > Guarantee that the on-disk timestamps will be no more than 24 hours
> > stale.
> >
> > +	unsigned short days_since_boot = jiffies / (HZ * 86400);
> 
> This seems to wrap every 49 days (assuming 32 bit jiffies and HZ==1000),
> so on-disk updates can be delayed indefinitely, assuming just the right
> delays between writes.

Good point, I'll fix this.

> Would it make sense to introduce days_since_boot as a global variable
> and avoid these issues? This would presumably also make update_time a
> few cycles faster (avoiding a division-by-constant), but not sure if
> that's important. And something of course needs to update
> days_since_boot, but that should be doable.

I can do this fairly simply like this:

	get_monotonic_boottime(&uptime);
	daycode = uptime.tv_sec / (HZ * 86400);

and we only need to do this if lazytime is set, and the inode isn't
marked as I_DIRTY_TIME:

	if ((inode->i_sb->s_flags & MS_LAZYTIME) &&
	    !(flags & S_VERSION)) {
		if (inode->i_state & I_DIRTY_TIME)
			return 0;
		get_monotonic_boottime(&uptime);
		daycode = do_div64(uptime.tv_sec do_div, (HZ * 86400));
		if (!inode->i_ts_dirty_day ||
		    inode->i_ts_dirty_day == daycode) {
			spin_lock(&inode->i_lock);
			inode->i_state |= I_DIRTY_TIME;
			spin_unlock(&inode->i_lock);
			inode->i_ts_dirty_day = daycode;
			return 0;
		}
	}

So I'm not entirely sure it's worth it to create a global variable for
days since boot; I've been runnin with this patch in my laptop, we
wouldn't be triggering the get_monotonic_bootime() function all that
often.  (Since once the dirty_time flg is set, we don't need to check
about whether we need to set it again.)  And if we *did* care, it
would be simple enough to use a static counter which only recalculates
daycode every 30 or 60 minutes.


Cheers,

							- Ted