| Message ID | 20230609175341.1618652-1-clm@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Btrfs: can_nocow_file_extent should pass down args->strict from callers | expand |
On Fri, Jun 9, 2023 at 7:17 PM Chris Mason <clm@fb.com> wrote: > > 619104ba453ad0 changed our call to btrfs_cross_ref_exist() to always > pass false for the 'strict' parameter. We're passing this down through > the stack so that we can do a full check for cross references during > swapfile activation. > > With strict always false, this test fails: > > btrfs subvol create swappy > chattr +C swappy > fallocate -l1G swappy/swapfile > chmod 600 swappy/swapfile > mkswap swappy/swapfile > > btrfs subvol snap swappy swapsnap > btrfs subvol del -C swapsnap > > btrfs fi sync / > sync;sync;sync > > swapon swappy/swapfile > > The fix is to just use args->strict, and everyone except swapfile > activation is passing false. > > Fixes: 619104ba453ad0 ("btrfs: move common NOCOW checks against a file extent into a helper") > Signed-off-by: Chris Mason <clm@fb.com> Looks good, thanks. Reviewed-by: Filipe Manana <fdmanana@suse.com> > --- > fs/btrfs/inode.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 19c707bc8801..e57d9969bd71 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -1864,7 +1864,7 @@ static int can_nocow_file_extent(struct btrfs_path *path, > > ret = btrfs_cross_ref_exist(root, btrfs_ino(inode), > key->offset - args->extent_offset, > - args->disk_bytenr, false, path); > + args->disk_bytenr, args->strict, path); > WARN_ON_ONCE(ret > 0 && is_freespace_inode); > if (ret != 0) > goto out; > -- > 2.34.1 >
On Fri, Jun 09, 2023 at 10:53:41AM -0700, Chris Mason wrote: > 619104ba453ad0 changed our call to btrfs_cross_ref_exist() to always > pass false for the 'strict' parameter. We're passing this down through > the stack so that we can do a full check for cross references during > swapfile activation. > > With strict always false, this test fails: > > btrfs subvol create swappy > chattr +C swappy > fallocate -l1G swappy/swapfile > chmod 600 swappy/swapfile > mkswap swappy/swapfile > > btrfs subvol snap swappy swapsnap > btrfs subvol del -C swapsnap > > btrfs fi sync / > sync;sync;sync > > swapon swappy/swapfile > > The fix is to just use args->strict, and everyone except swapfile > activation is passing false. > > Fixes: 619104ba453ad0 ("btrfs: move common NOCOW checks against a file extent into a helper") > Signed-off-by: Chris Mason <clm@fb.com> Added to misc-next, thanks.
On Fri, Jun 9, 2023 at 7:17 PM Chris Mason <clm@fb.com> wrote: > > 619104ba453ad0 changed our call to btrfs_cross_ref_exist() to always > pass false for the 'strict' parameter. We're passing this down through > the stack so that we can do a full check for cross references during > swapfile activation. > > With strict always false, this test fails: > > btrfs subvol create swappy > chattr +C swappy > fallocate -l1G swappy/swapfile > chmod 600 swappy/swapfile > mkswap swappy/swapfile > > btrfs subvol snap swappy swapsnap > btrfs subvol del -C swapsnap > > btrfs fi sync / > sync;sync;sync > > swapon swappy/swapfile Btw, I just sent a test case for this and to verify as well that we can't activate swap files if there are snapshots. https://lore.kernel.org/linux-btrfs/5417083e8e23a1553f428608d02a07aae21b9e53.1687262391.git.fdmanana@suse.com/ Clearly something we lacked test coverage. > > The fix is to just use args->strict, and everyone except swapfile > activation is passing false. > > Fixes: 619104ba453ad0 ("btrfs: move common NOCOW checks against a file extent into a helper") > Signed-off-by: Chris Mason <clm@fb.com> > --- > fs/btrfs/inode.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 19c707bc8801..e57d9969bd71 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -1864,7 +1864,7 @@ static int can_nocow_file_extent(struct btrfs_path *path, > > ret = btrfs_cross_ref_exist(root, btrfs_ino(inode), > key->offset - args->extent_offset, > - args->disk_bytenr, false, path); > + args->disk_bytenr, args->strict, path); > WARN_ON_ONCE(ret > 0 && is_freespace_inode); > if (ret != 0) > goto out; > -- > 2.34.1 >
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 19c707bc8801..e57d9969bd71 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -1864,7 +1864,7 @@ static int can_nocow_file_extent(struct btrfs_path *path, ret = btrfs_cross_ref_exist(root, btrfs_ino(inode), key->offset - args->extent_offset, - args->disk_bytenr, false, path); + args->disk_bytenr, args->strict, path); WARN_ON_ONCE(ret > 0 && is_freespace_inode); if (ret != 0) goto out;
619104ba453ad0 changed our call to btrfs_cross_ref_exist() to always pass false for the 'strict' parameter. We're passing this down through the stack so that we can do a full check for cross references during swapfile activation. With strict always false, this test fails: btrfs subvol create swappy chattr +C swappy fallocate -l1G swappy/swapfile chmod 600 swappy/swapfile mkswap swappy/swapfile btrfs subvol snap swappy swapsnap btrfs subvol del -C swapsnap btrfs fi sync / sync;sync;sync swapon swappy/swapfile The fix is to just use args->strict, and everyone except swapfile activation is passing false. Fixes: 619104ba453ad0 ("btrfs: move common NOCOW checks against a file extent into a helper") Signed-off-by: Chris Mason <clm@fb.com> --- fs/btrfs/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)