Re: [PATCH] btrfs: detect nocow for swap after snapshot delete

[Boris Burkov <boris@bur.io>]

On Wed, Aug 19, 2020 at 04:25:48PM +0200, David Sterba wrote:
> On Wed, Aug 19, 2020 at 12:59:28PM +0100, Graham Cobb wrote:
> > On 19/08/2020 12:29, David Sterba wrote:
> > > How often could the snapshot deletion and swapfile activation happen at
> > > the same time? Snapshotting subvolume with the swapfile requires
> > > deactivation, snapshot/send/whatever and then activation. This sounds
> > > like a realistic usecase.
> > 
> > It is very likely when the swapfile is one that is only used
> > occasionally (for example, when running a particular program which needs
> > a massive amount of virtual memory, or having to stop using a different
> > swapfile because a disk looks like it is starting to fail).
> > 
> > If the swapfile is not normally used, it is not unlikely it got
> > snapshotted (as part of a larger operation, presumably) while
> > deactivated. When the user tries to use it, they realise it isn't
> > working because it is snapshotted, so they delete the snapshot and then
> > immediately try to activate it again -- causing confusion when it still
> > fails.
> 
> That makes sense from user POV. I still don't uderstand if it's
> sufficient to commit the transaction deleting the snapshot or if it's
> necessary to wait until the subvolume is completely cleaned.
> 
> The former would require 'btrfs subvol delte -c /snapshot' while the
> latter needs the id of the subvolume and then
> 'btrfs subvol sync /path id'.

My reproduction has been:
create subvol
create swapfile in subvol
btrfs subvol snapshot subvol snapshot
btrfs subvol delete snapshot
sync/btrfs fi sync/waiting
swapon subvol/swapfile

Note that I haven't been touching the swapfile in any way after
initially creating it. Nor have I been turning swap on/off in any
coordinated way before/after the snapshot except to test after the
snapshot is deleted.

I tried both suggestions with and without the patch and saw that with
the patch, swapon reliably succeeds after 'btrfs subvol sync' and does
not reliably succeed even after 'btrfs subvol delete -c' or sync calls,
so it seems that we need the subvolume to be completely cleaned. Without
the patch, after both 'btrfs subvol delete -c' and after 'btrfs subvol
sync', the swapon fails. Thanks for the review, and testing suggestions!