Re: [PATCH 1/1] writeback fix bdi thread race in mark_inode_dirty

[Jan Kara <jack@suse.cz>]

On Fri 19-07-13 11:05:40, Srinivas Eeda wrote:
> In __mark_inode_dirty, a process checks !wb_has_dirty_io outside of list_lock
> spinlock. This could cause a race, where process sees that list has dirty io
> and decides not wake up bdi thread and waits for spinlock to add to dirty list.
> Right at this time bdi_writeback_workfn finished write-back on last inode.
> It sees the list is empty and ends. Process could now get the spinlock and
> add inode to dirty list and doesn't wakeup bdi thread. Future calls to
> __mark_inode_dirty also do not wake up the thread because list is not empty
> any more.
> 
> Fix is to get wb.list_lock spinlock before checking the dirty list
> 
> Signed-off-by: Srinivas Eeda <srinivas.eeda@oracle.com>
  Good catch! The patch looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
> ---
>  fs/fs-writeback.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 68851ff..a5abc6c 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1173,6 +1173,9 @@ void __mark_inode_dirty(struct inode *inode, int flags)
>  			bool wakeup_bdi = false;
>  			bdi = inode_to_bdi(inode);
>  
> +			spin_unlock(&inode->i_lock);
> +			spin_lock(&bdi->wb.list_lock);
> +
>  			if (bdi_cap_writeback_dirty(bdi)) {
>  				WARN(!test_bit(BDI_registered, &bdi->state),
>  				     "bdi-%s not registered\n", bdi->name);
> @@ -1187,8 +1190,6 @@ void __mark_inode_dirty(struct inode *inode, int flags)
>  					wakeup_bdi = true;
>  			}
>  
> -			spin_unlock(&inode->i_lock);
> -			spin_lock(&bdi->wb.list_lock);
>  			inode->dirtied_when = jiffies;
>  			list_move(&inode->i_wb_list, &bdi->wb.b_dirty);
>  			spin_unlock(&bdi->wb.list_lock);
> -- 
> 1.5.6.5
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR