[add linux-xfs and xfsprogs maintainer to cc]
On Thu, Jun 11, 2020 at 10:11:55AM +0200, Jan Kara wrote:
> The only use of I_DIRTY_TIME_EXPIRE is to detect in
> __writeback_single_inode() that inode got there because flush worker
> decided it's time to writeback the dirty inode time stamps (either
> because we are syncing or because of age). However we can detect this
> directly in __writeback_single_inode() and there's no need for the
> strange propagation with I_DIRTY_TIME_EXPIRE flag.
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
> fs/ext4/inode.c | 2 +-
> fs/fs-writeback.c | 28 +++++++++++-----------------
> fs/xfs/libxfs/xfs_trans_inode.c | 4 ++--
Urrk, so I only just noticed this when I rebased my development tree
onto 5.9-rc3. If you're going to change things in fs/xfs/, please cc
the xfs list to keep us in the loop. Changes to fs/xfs/libxfs/ have to
be ported to userspace.
--D
> include/linux/fs.h | 1 -
> include/trace/events/writeback.h | 1 -
> 5 files changed, 14 insertions(+), 22 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 40ec5c7ef0d3..4db497f02ffb 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4887,7 +4887,7 @@ static void __ext4_update_other_inode_time(struct super_block *sb,
> (inode->i_state & I_DIRTY_TIME)) {
> struct ext4_inode_info *ei = EXT4_I(inode);
>
> - inode->i_state &= ~(I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED);
> + inode->i_state &= ~I_DIRTY_TIME;
> spin_unlock(&inode->i_lock);
>
> spin_lock(&ei->i_raw_lock);
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index ae17d64a3e18..149227160ff0 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1238,7 +1238,7 @@ static bool inode_dirtied_after(struct inode *inode, unsigned long t)
> */
> static int move_expired_inodes(struct list_head *delaying_queue,
> struct list_head *dispatch_queue,
> - int flags, unsigned long dirtied_before)
> + unsigned long dirtied_before)
> {
> LIST_HEAD(tmp);
> struct list_head *pos, *node;
> @@ -1254,8 +1254,6 @@ static int move_expired_inodes(struct list_head *delaying_queue,
> list_move(&inode->i_io_list, &tmp);
> moved++;
> spin_lock(&inode->i_lock);
> - if (flags & EXPIRE_DIRTY_ATIME)
> - inode->i_state |= I_DIRTY_TIME_EXPIRED;
> inode->i_state |= I_SYNC_QUEUED;
> spin_unlock(&inode->i_lock);
> if (sb_is_blkdev_sb(inode->i_sb))
> @@ -1303,11 +1301,11 @@ static void queue_io(struct bdi_writeback *wb, struct wb_writeback_work *work,
>
> assert_spin_locked(&wb->list_lock);
> list_splice_init(&wb->b_more_io, &wb->b_io);
> - moved = move_expired_inodes(&wb->b_dirty, &wb->b_io, 0, dirtied_before);
> + moved = move_expired_inodes(&wb->b_dirty, &wb->b_io, dirtied_before);
> if (!work->for_sync)
> time_expire_jif = jiffies - dirtytime_expire_interval * HZ;
> moved += move_expired_inodes(&wb->b_dirty_time, &wb->b_io,
> - EXPIRE_DIRTY_ATIME, time_expire_jif);
> + time_expire_jif);
> if (moved)
> wb_io_lists_populated(wb);
> trace_writeback_queue_io(wb, work, dirtied_before, moved);
> @@ -1483,18 +1481,14 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
> spin_lock(&inode->i_lock);
>
> dirty = inode->i_state & I_DIRTY;
> - if (inode->i_state & I_DIRTY_TIME) {
> - if ((dirty & I_DIRTY_INODE) ||
> - wbc->sync_mode == WB_SYNC_ALL ||
> - unlikely(inode->i_state & I_DIRTY_TIME_EXPIRED) ||
> - unlikely(time_after(jiffies,
> - (inode->dirtied_time_when +
> - dirtytime_expire_interval * HZ)))) {
> - dirty |= I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED;
> - trace_writeback_lazytime(inode);
> - }
> - } else
> - inode->i_state &= ~I_DIRTY_TIME_EXPIRED;
> + if ((inode->i_state & I_DIRTY_TIME) &&
> + ((dirty & I_DIRTY_INODE) ||
> + wbc->sync_mode == WB_SYNC_ALL || wbc->for_sync ||
> + time_after(jiffies, inode->dirtied_time_when +
> + dirtytime_expire_interval * HZ))) {
> + dirty |= I_DIRTY_TIME;
> + trace_writeback_lazytime(inode);
> + }
> inode->i_state &= ~dirty;
>
> /*
> diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c
> index b5dfb6654842..1b4df6636944 100644
> --- a/fs/xfs/libxfs/xfs_trans_inode.c
> +++ b/fs/xfs/libxfs/xfs_trans_inode.c
> @@ -96,9 +96,9 @@ xfs_trans_log_inode(
> * to log the timestamps, or will clear already cleared fields in the
> * worst case.
> */
> - if (inode->i_state & (I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED)) {
> + if (inode->i_state & I_DIRTY_TIME) {
> spin_lock(&inode->i_lock);
> - inode->i_state &= ~(I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED);
> + inode->i_state &= ~I_DIRTY_TIME;
> spin_unlock(&inode->i_lock);
> }
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 48556efcdcf0..45eadf5bea5d 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2178,7 +2178,6 @@ static inline void kiocb_clone(struct kiocb *kiocb, struct kiocb *kiocb_src,
> #define I_DIO_WAKEUP (1 << __I_DIO_WAKEUP)
> #define I_LINKABLE (1 << 10)
> #define I_DIRTY_TIME (1 << 11)
> -#define I_DIRTY_TIME_EXPIRED (1 << 12)
> #define I_WB_SWITCH (1 << 13)
> #define I_OVL_INUSE (1 << 14)
> #define I_CREATING (1 << 15)
> diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
> index 7565dcd59697..e7cbccc7c14c 100644
> --- a/include/trace/events/writeback.h
> +++ b/include/trace/events/writeback.h
> @@ -20,7 +20,6 @@
> {I_CLEAR, "I_CLEAR"}, \
> {I_SYNC, "I_SYNC"}, \
> {I_DIRTY_TIME, "I_DIRTY_TIME"}, \
> - {I_DIRTY_TIME_EXPIRED, "I_DIRTY_TIME_EXPIRED"}, \
> {I_REFERENCED, "I_REFERENCED"} \
> )
>
> --
> 2.16.4
>
