On Mon 16-03-15 15:34:12, Andreas Dilger wrote: > On Mar 16, 2015, at 1:14 PM, Theodore Ts'o <tytso@xxxxxxx> 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. > > The drawback here is that this adds another 8 bytes to every inode for > a field of marginal value, since this is only important for the rare > case of a file that is being dirtied continuously. Yes. > I wonder if something more lightweight could be added to avoid this > problem? For example, we only care about this case if it has been > going on for more than the lazytime interval (about a day), so the > inode could store a 16-bit i_dirtied_time_when that is approximately > (jiffies >> bits_in_a_half_a_day) and only check time_after() that. > The __u16 could fit into some existing hole (e.g. after i_bytes on my > kernel) and avoid expanding the size of the inode at all. > > The remaining high bits of i_dirtied_time_when would be irrelevant, since > a __u16 of half-days is about 80 years, so it would be enough to compare: > > > time_after(i_dirtied_time_when, (__u16)(jiffies >> bits_in_half_a_day)) > > > A day is 86400s, so 43200s is close to (1 << 22) jiffies for HZ=100, and > (1 << 25) jiffies is about 3/8 of a day for HZ=1000. Since the exact > times for inode writeout don't matter very much here, having only shifts > to convert jiffies to i_dirtied_time_when in the kernel is better I think. Yes, something like this should be possible. But I wanted that to happen as a separate patch once we have everything working correctly. The code is subtle enough that I didn't want Ted to complicate it with further optimizations initially. > Minor issue, is there a good reason why dirtied_time_when doesn't have an > "i_" prefix? I guess it's matching with dirtied_when which doesn't have i_ prefix just because noone added it initially. I don't really care either way. Honza -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html