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:34:12, Andreas Dilger wrote:
> On Mar 16, 2015, at 1:14 PM, Theodore Ts'o <tytso@mit.edu> 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@suse.cz>
SUSE Labs, CR