On Fri, Jan 08, 2021 at 10:01:33AM +0100, Christoph Hellwig wrote: > > + /* > > + * If inode has dirty timestamps and we need to write them, call > > + * mark_inode_dirty_sync() to notify filesystem about it. > > + */ > > + if (inode->i_state & I_DIRTY_TIME && > > + (wbc->for_sync || wbc->sync_mode == WB_SYNC_ALL || > > + time_after(jiffies, inode->dirtied_time_when + > > + dirtytime_expire_interval * HZ))) { > > If we're touching this area, it would be nice to split this condition > into a readable helper ala: > > static inline bool inode_needs_timestamp_sync(struct writeback_control *wbc, > struct inode *inode) > { > if (!(inode->i_state & I_DIRTY_TIME)) > return false; > if (wbc->for_sync || wbc->sync_mode == WB_SYNC_ALL) > return true; > return time_after(jiffies, inode->dirtied_time_when + > dirtytime_expire_interval * HZ); > } I didn't end up doing this since it would only be called once, and IMO it's more readable to keep it inlined next to the comment that explains what's going on. Especially considering the later patch that drops the check for wbc->for_sync. - Eric