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-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html