Hi Jan,
On Thu, Jun 11, 2020 at 10:12 AM Jan Kara <jack@xxxxxxx> wrote:
> Reported-by: Martijn Coenen <maco@xxxxxxxxxxx>
> Fixes: 0ae45f63d4ef ("vfs: add support for a lazytime mount option")
> CC: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jan Kara <jack@xxxxxxx>
Thanks again for the fix. We've been running this (well, v1) for over
2 weeks across at least ~1000 Android devices with different kernel
versions, and I can confirm we haven't run into the issue this intends
to fix, or any other issues for that matter. The patch LGTM as well.
Reviewed-by: Martijn Coenen <maco@xxxxxxxxxxx>
Tested-by: Martijn Coenen <maco@xxxxxxxxxxx>
> ---
> fs/fs-writeback.c | 17 ++++++++++++-----
> include/linux/fs.h | 8 ++++++--
> 2 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index ff0b18331590..f470c10641c5 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -146,6 +146,7 @@ static void inode_io_list_del_locked(struct inode *inode,
> assert_spin_locked(&wb->list_lock);
> assert_spin_locked(&inode->i_lock);
>
> + inode->i_state &= ~I_SYNC_QUEUED;
> list_del_init(&inode->i_io_list);
> wb_io_lists_depopulated(wb);
> }
> @@ -1187,6 +1188,7 @@ static void redirty_tail_locked(struct inode *inode, struct bdi_writeback *wb)
> inode->dirtied_when = jiffies;
> }
> inode_io_list_move_locked(inode, wb, &wb->b_dirty);
> + inode->i_state &= ~I_SYNC_QUEUED;
> }
>
> static void redirty_tail(struct inode *inode, struct bdi_writeback *wb)
> @@ -1262,8 +1264,11 @@ static int move_expired_inodes(struct list_head *delaying_queue,
> break;
> list_move(&inode->i_io_list, &tmp);
> moved++;
> + spin_lock(&inode->i_lock);
> if (flags & EXPIRE_DIRTY_ATIME)
> - set_bit(__I_DIRTY_TIME_EXPIRED, &inode->i_state);
> + 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))
> continue;
> if (sb && sb != inode->i_sb)
> @@ -1438,6 +1443,7 @@ static void requeue_inode(struct inode *inode, struct bdi_writeback *wb,
> } else if (inode->i_state & I_DIRTY_TIME) {
> inode->dirtied_when = jiffies;
> inode_io_list_move_locked(inode, wb, &wb->b_dirty_time);
> + inode->i_state &= ~I_SYNC_QUEUED;
> } else {
> /* The inode is clean. Remove from writeback lists. */
> inode_io_list_del_locked(inode, wb);
> @@ -2301,11 +2307,12 @@ void __mark_inode_dirty(struct inode *inode, int flags)
> inode->i_state |= flags;
>
> /*
> - * If the inode is being synced, just update its dirty state.
> - * The unlocker will place the inode on the appropriate
> - * superblock list, based upon its state.
> + * If the inode is queued for writeback by flush worker, just
> + * update its dirty state. Once the flush worker is done with
> + * the inode it will place it on the appropriate superblock
> + * list, based upon its state.
> */
> - if (inode->i_state & I_SYNC)
> + if (inode->i_state & I_SYNC_QUEUED)
> goto out_unlock_inode;
>
> /*
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 19ef6c88c152..48556efcdcf0 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2157,6 +2157,10 @@ static inline void kiocb_clone(struct kiocb *kiocb, struct kiocb *kiocb_src,
> *
> * I_DONTCACHE Evict inode as soon as it is not used anymore.
> *
> + * I_SYNC_QUEUED Inode is queued in b_io or b_more_io writeback lists.
> + * Used to detect that mark_inode_dirty() should not move
> + * inode between dirty lists.
> + *
> * Q: What is the difference between I_WILL_FREE and I_FREEING?
> */
> #define I_DIRTY_SYNC (1 << 0)
> @@ -2174,12 +2178,12 @@ 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 12
> -#define I_DIRTY_TIME_EXPIRED (1 << __I_DIRTY_TIME_EXPIRED)
> +#define I_DIRTY_TIME_EXPIRED (1 << 12)
> #define I_WB_SWITCH (1 << 13)
> #define I_OVL_INUSE (1 << 14)
> #define I_CREATING (1 << 15)
> #define I_DONTCACHE (1 << 16)
> +#define I_SYNC_QUEUED (1 << 17)
>
> #define I_DIRTY_INODE (I_DIRTY_SYNC | I_DIRTY_DATASYNC)
> #define I_DIRTY (I_DIRTY_INODE | I_DIRTY_PAGES)
> --
> 2.16.4
>
