Re: [PATCH] xfs: fix deadlock when set label online

[yangerkun <yangerkun@huaweicloud.com>]

在 2023/6/27 5:45, Dave Chinner 写道:
> On Mon, Jun 26, 2023 at 09:15:42PM +0800, yangerkun wrote:
>> From: yangerkun <yangerkun@huawei.com>
>>
>> Combine use of xfs_trans_hold and xfs_trans_set_sync in xfs_sync_sb_buf
>> can trigger a deadlock once shutdown happened concurrently. xlog_ioend_work
>> will first unpin the sb(which stuck with xfs_buf_lock), then wakeup
>> xfs_sync_sb_buf. However, xfs_sync_sb_buf never get the chance to unlock
>> sb until been wakeup by xlog_ioend_work.
>>
>> xfs_sync_sb_buf
>>    xfs_trans_getsb // lock sb buf
>>    xfs_trans_bhold // sb buf keep lock until success commit
>>    xfs_trans_commit
>>    ...
>>      xfs_log_force_seq
>>        xlog_force_lsn
>>          xlog_wait_on_iclog
>>            xlog_wait(&iclog->ic_force_wait... // shutdown happened
>>    xfs_buf_relse // unlock sb buf
>>
>> xlog_ioend_work
>>    xlog_force_shutdown
>>      xlog_state_shutdown_callbacks
>>        xlog_cil_process_committed
>>          xlog_cil_committed
>>          ...
>>          xfs_buf_item_unpin
>>            xfs_buf_lock // deadlock
>>        wake_up_all(&iclog->ic_force_wait)
>>
>> xfs_ioc_setlabel use xfs_sync_sb_buf to make sure userspace will see the
>> change for sb immediately. We can simply call xfs_ail_push_all_sync to
>> do this and sametime fix the deadlock.
> 
> Why is this deadlock specific to the superblock buffer?

Hi Dave,

Thanks a lot for your revirew! We find this problem when do some code 
reading(which can help us to fix another growfs bug). And then reproduce 
it easily when we set label online frequently with IO error inject at 
the sametime.

> 
> Can't any buffer that is held locked over a synchronous transaction
> commit deadlock during a shutdown like this?

After check all place use xfs_buf_bhold, it seems xfs_sync_sb_buf is the 
only convict that combine use xfs_trans_hold and xfs_trans_set_sync(I'm 
not familiar with xfs yet, so I may have some problems with my code 
check)...

Thanks,
Yang Erkun.
> 
> Cheers,
> 
> Dave.