Re: Linux swap file not activating after reboot

[Omar Sandoval <osandov@osandov.com>]

On Thu, Jan 16, 2020 at 07:55:39PM +0100, David Sterba wrote:
> On Wed, Jan 15, 2020 at 04:34:02PM -0800, Michael Ruiz wrote:
> > Hi,
> >  I have a //@swap subvolume and i have a swapfile within it. I mount the 
> > subvolume like such in fstab:
> > 
> > `rw,ssd,nofail,noautodefrag,nodatacow,nodatasum,subvolid=1234,subvol=/@swap`
> > 
> > It mounts correctly, but 1/15/20 4:20 PM kernel I get: 
> > 
> > `BTRFS warning (device dm-0): swapfile must not be copy-on-write`
> 
> There are two reasons why the message is printed, one is when the file
> does not have the C attribute and another one when the the existing file
> extents need to be COWed (same case as if the file is NOCOW and has been
> snapshotted).
> 
> Plain reboot will not change the C attribute, so either there's a
> snapshot of /@snap or the check of a used swapfile is wrong.
> 
> I tested it here, a swapfile that got almost full after a stress test,
> followed by reboot and swapon (without any change to the file) was ok.
> 
> Doing a snapshot and swapon resulted in the message you saw.
> 
> After deleting the snapshot and waiting until it gets cleaned, swapon
> did not activate the file anymore. Filefrag or fiemap don't report any
> shared extents so here I' expect that the file should be in a valid
> state for swapon.
> 
> Omar, any ideas?

Hm, we're hitting this check in can_nocow_extent():

        if (btrfs_file_extent_generation(leaf, fi) <=
            btrfs_root_last_snapshot(&root->root_item))

That check was added in 78d4295b1eee ("btrfs: lift some
btrfs_cross_ref_exist checks in nocow path") as an optimization. Even if
we comment that out, we'll hit the similar check in
btrfs_cross_ref_exist():

        /* If extent created before last snapshot => it's definitely shared */
        if (btrfs_extent_generation(leaf, ei) <=
            btrfs_root_last_snapshot(&root->root_item))

That's not quite right in exactly this case that the snapshot has been
deleted. Apparently we've been doing unnecessary COW for this case. I'll
need to think about how to safely avoid these checks without too much of
a performance hit.

Thanks for the report!