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@xxxxxxxxxx>
Good catch! The patch looks good. You can add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
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@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
