On Fri 15-09-23 22:51:31, Chunhai Guo wrote:
> When writing back an inode and performing an fsync on it concurrently, a
> deadlock issue may arise as shown below. In each writeback iteration, a
> clean inode is requeued to the wb->b_dirty queue due to non-zero
> pages_skipped, without anything actually being written. This causes an
> infinite loop and prevents the plug from being flushed, resulting in a
> deadlock. We now avoid requeuing the clean inode to prevent this issue.
>
> wb_writeback fsync (inode-Y)
> blk_start_plug(&plug)
> for (;;) {
> iter i-1: some reqs with page-X added into plug->mq_list // f2fs node page-X with PG_writeback
> filemap_fdatawrite
> __filemap_fdatawrite_range // write inode-Y with sync_mode WB_SYNC_ALL
> do_writepages
> f2fs_write_data_pages
> __f2fs_write_data_pages // wb_sync_req[DATA]++ for WB_SYNC_ALL
> f2fs_write_cache_pages
> f2fs_write_single_data_page
> f2fs_do_write_data_page
> f2fs_outplace_write_data
> f2fs_update_data_blkaddr
> f2fs_wait_on_page_writeback
> wait_on_page_writeback // wait for f2fs node page-X
> iter i:
> progress = __writeback_inodes_wb(wb, work)
> . writeback_sb_inodes
> . __writeback_single_inode // write inode-Y with sync_mode WB_SYNC_NONE
> . . do_writepages
> . . f2fs_write_data_pages
> . . . __f2fs_write_data_pages // skip writepages due to (wb_sync_req[DATA]>0)
> . . . wbc->pages_skipped += get_dirty_pages(inode) // wbc->pages_skipped = 1
> . if (!(inode->i_state & I_DIRTY_ALL)) // i_state = I_SYNC | I_SYNC_QUEUED
> . total_wrote++; // total_wrote = 1
> . requeue_inode // requeue inode-Y to wb->b_dirty queue due to non-zero pages_skipped
> if (progress) // progress = 1
> continue;
> iter i+1:
> queue_io
> // similar process with iter i, infinite for-loop !
> }
> blk_finish_plug(&plug) // flush plug won't be called
>
> Signed-off-by: Chunhai Guo <guochunhai@xxxxxxxx>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 969ce991b0b0..c1af01b2c42d 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1535,10 +1535,15 @@ static void requeue_inode(struct inode *inode, struct bdi_writeback *wb,
>
> if (wbc->pages_skipped) {
> /*
> - * writeback is not making progress due to locked
> - * buffers. Skip this inode for now.
> + * Writeback is not making progress due to locked buffers.
> + * Skip this inode for now. Although having skipped pages
> + * is odd for clean inodes, it can happen for some
> + * filesystems so handle that gracefully.
> */
> - redirty_tail_locked(inode, wb);
> + if (inode->i_state & I_DIRTY_ALL)
> + redirty_tail_locked(inode, wb);
> + else
> + inode_cgwb_move_to_attached(inode, wb);
> return;
> }
>
> --
> 2.25.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
