Re: [PATCH 0/5] remove BUG_ON()s in btrfs_close_one_device()

[David Sterba <dsterba@suse.cz>]

On Tue, Nov 12, 2019 at 01:24:11PM +0100, Johannes Thumshirn wrote:
> This series attempts to remove the BUG_ON()s in btrfs_close_one_device().
> Therefore some reorganization of btrfs_close_one_device() and
> close_fs_devices() was needed.
> 
> Forthermore a new BUG_ON() had to be temporarily introduced but is removed
> again in the last patch of theis series.

Yeah, that's fine.

> Although it is generally legal to return -ENOMEM on umount(2), the error
> handling up until close_ctree() as neither close_ctree() nor btrfs_put_super()
> would be able to handle the error.
> 
> This series has passed fstests without any deviation from the baseline and
> also the new error handling was tested via error injection using this snippet:

This BUG_ON is one of the syzbot reports so once this patchset is
reviewed, we can ask for a retest.

I have a WIP fix that tries to avoid ENOMEM completely as it's not
strictly necessary. The empty device is like a placeholder in the list
while the real device is RCU-removed. The fix is to mark the device as
"being deleted" so it can be skipped but this is more intrusive and more
error prone than handling the error.

As this is not a frequent error at all, syzbot artifically limits the
amount of memory, otherwise we haven't seen the ENOMEM in practice. So
the fix is IMO sufficient for now.