On Wed, Jul 03, 2019 at 12:45:49PM +0300, Nikolay Borisov wrote:
> Recent FITRIM work, namely bbbf7243d62d ("btrfs: combine device update
> operations during transaction commit") combined the way certain
> operations are recoded in a transaction. As a result an ASSERT was added
> in dev_replace_finish to ensure the new code works correctly.
> Unfortunately I got reports that it's possible to trigger the assert,
> meaning that during a device replace it's possible to have an unfinished
> chunk allocation on the source device.
>
> This is supposed to be prevented by the fact that a transaction is
> committed before finishing the replace oepration and alter acquiring the
> chunk mutex. This is not sufficient since by the time the transaction is
> committed and the chunk mutex acquired it's possible to allocate a chunk
> depending on the workload being executed on the replaced device. This
> bug has been present ever since device replace was introduced but there
> was never code which checks for it.
>
> The correct way to fix is to ensure that there is no pending device
> modification operation when the chunk mutex is acquire and if there is
> repeat transaction commit. Unfortunately it's not possible to just
> exclude the source device from btrfs_fs_devices::dev_alloc_list since
> this causes ENOSPC to be hit in transaction commit.
>
> Fixing that in another way would need to add special cases to handle the
> last writes and forbid new ones. The looped transaction fix is more
> obvious, and can be easily backported. The runtime of dev-replace is
> long so there's no noticeable delay caused by that.
>
> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx>
> ---
>
> Hello Greg,
>
> Please merge the following backport of upstream commit debd1c065d2037919a7da67baf55cc683fee09f0
> to 4.4.y stable branch.
Thanks for all of these, now queued up.
greg k-h
