* [PATCH 0/1 NOT FOR MERGE] Basic subdir tracking in nlink
@ 2020-08-27 15:04 Nikolay Borisov
2020-08-27 15:04 ` [PATCH] btrfs/218: Test for i_nlink tracking Nikolay Borisov
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Nikolay Borisov @ 2020-08-27 15:04 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
Here are the various bits and pieces needed to have a full-fledged subdir
tracking in nlink. The kernel code is complete and the accompanying fstests
proves that. Nevertheless, progs would need a bit more work, with the attached
progs patch I get only the following tests failing due to progs' nlink code:
generic/077, generic/107, generic/498 and generic/547
At this point I won't be working anymore on this unless there is a really
compelling argument to do so, however I'm sending the patches now so that if
anyone should feel inclined to finish the work they they can base their efforts
on mine. At this point what's left is:
* Decide how old kernels are supposed to be supported - tree-checker won't allow
them too mount an fs whose hard link counter for directories is != 1. This would
either require an incompat_ro flag or a coordinated effort to backport the patch
to LTS kernels.
* The remaining fstests failures need to be investigated and fixed.
* btrfs-progs lowmem mode would likely consider a filesystem broken so it will
also need to be adjusted.
Nikolay Borisov (1):
btrfs: Track subdirectories in nlink
fs/btrfs/inode.c | 13 +++++++++++--
fs/btrfs/ioctl.c | 10 +++++++---
fs/btrfs/transaction.c | 12 ++++++++----
fs/btrfs/tree-checker.c | 7 +------
4 files changed, 27 insertions(+), 15 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] btrfs/218: Test for i_nlink tracking 2020-08-27 15:04 [PATCH 0/1 NOT FOR MERGE] Basic subdir tracking in nlink Nikolay Borisov @ 2020-08-27 15:04 ` Nikolay Borisov 2020-08-27 15:04 ` [PATCH 1/1] btrfs: Track subdirectories in nlink Nikolay Borisov 2020-08-27 15:04 ` [PATCH] btrfs-progs: check: Support precise nlink tracking Nikolay Borisov 2 siblings, 0 replies; 5+ messages in thread From: Nikolay Borisov @ 2020-08-27 15:04 UTC (permalink / raw) To: linux-btrfs; +Cc: Nikolay Borisov Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- tests/btrfs/218 | 170 ++++++++++++++++++++++++++++++++++++++++++++ tests/btrfs/218.out | 2 + tests/btrfs/group | 1 + 3 files changed, 173 insertions(+) create mode 100755 tests/btrfs/218 create mode 100644 tests/btrfs/218.out diff --git a/tests/btrfs/218 b/tests/btrfs/218 new file mode 100755 index 000000000000..571eca8a20e2 --- /dev/null +++ b/tests/btrfs/218 @@ -0,0 +1,170 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2020 SUSE Linux Products GmbH. All Rights Reserved. +# +# FS QA Test 218 +# +# Test that btrfs is correctly keeping track of link count for directories. +# +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 15 + +_cleanup() +{ + cd / + rm -f $tmp.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# real QA test starts here + +# Modify as appropriate. +_supported_fs btrfs +_supported_os Linux +_require_test + +_scratch_mkfs > /dev/null 2>&1 +_scratch_mount + +# Ensure we have sane start value of 1 +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 1 ]; then + echo "Unexpected default link count: $linkcount expected: 1" +fi + +# moving/copying/deleting files shouldn't affect link count +touch $SCRATCH_MNT/foo +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 1 ]; then + echo "Unexpected link count after file creation: $linkcount expected: 1" +fi + +cp $SCRATCH_MNT/foo $SCRATCH_MNT/bar +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 1 ]; then + echo "Unexpected link count after file copy: $linkcount expected: 1" +fi + +rm $SCRATCH_MNT/bar +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 1 ]; then + echo "Unexpected link count after file delete: $linkcount expected: 1" +fi + +# move the file to oblivion +mv $SCRATCH_MNT/foo /dev/null +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 1 ]; then + echo "Unexpected link count after file move: $linkcount expected: 1" +fi + +# Add a dir and verify link count is incremented +mkdir $SCRATCH_MNT/foo +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 2 ]; then + echo "Unexpected link count after dir create: $linkcount expected: 2" +fi + +# Check if copy also increments the link count +cp -r $SCRATCH_MNT/foo $SCRATCH_MNT/bar +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 3 ]; then + echo "Unexpected link count after dir copy: $linkcount expected: 3" +fi + +# Now see if rename decrements the count of the src and increments the count +# of the dst +mv $SCRATCH_MNT/bar $SCRATCH_MNT/foo/ +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 2 ]; then + echo "Unexpected link count at src after dir move: $linkcount expected: 2" +fi + + +linkcount=$(stat -c %h $SCRATCH_MNT/foo) +if [ $linkcount != 2 ]; then + echo "Unexpected link count at dst after dir move: $linkcount expected: 2" +fi + +# Now delete a directory and see if count is correctly decremented +mkdir $SCRATCH_MNT/bar +rmdir $SCRATCH_MNT/bar +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 2 ]; then + echo "Unexpected link count after dir removal: $linkcount expected: 2" +fi + +# Now create a subvolume, it's also a dir +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/subvol > /dev/null +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 3 ]; then + echo "Unexpected link count after subvol create: $linkcount expected: 3" +fi + +mv $SCRATCH_MNT/subvol $SCRATCH_MNT/subvol1 +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 3 ]; then + echo "Unexpected link count after subvol move in same dir: $linkcount expected: 3" +fi + +# Move subvolume and see if src/dest counts are updated accordingly +mv $SCRATCH_MNT/subvol1 $SCRATCH_MNT/foo/ +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 2 ]; then + echo "Unexpected link count at src after subvol move: $linkcount expected: 2" +fi + +linkcount=$(stat -c %h $SCRATCH_MNT/foo) +if [ $linkcount != 3 ]; then + echo "Unexpected link count at dst after subvol move: $linkcount expected: 3" +fi + +# Now delete it +$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/foo/subvol1 > /dev/null +linkcount=$(stat -c %h $SCRATCH_MNT/foo) +if [ $linkcount != 2 ]; then + echo "Unexpected link count after subvol delete: $linkcount expected: 2" +fi + +# snapshots are created in a different path +$BTRFS_UTIL_PROG subvolume snapshot $SCRATCH_MNT $SCRATCH_MNT/snapshot1 > /dev/null +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 3 ]; then + echo "Unexpected link count after snapshot create: $linkcount expected: 3" +fi + +# move snapshot +mv $SCRATCH_MNT/snapshot1 $SCRATCH_MNT/foo +linkcount=$(stat -c %h $SCRATCH_MNT) +if [ $linkcount != 2 ]; then + echo "Unexpected link count after at src after snapshot move: $linkcount expected: 2" +fi + +linkcount=$(stat -c %h $SCRATCH_MNT/foo) +if [ $linkcount != 3 ]; then + echo "Unexpected link count after at dst after snapshot move: $linkcount expected: 2" +fi + +# delete snapshot +$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/foo/snapshot1 > /dev/null +linkcount=$(stat -c %h $SCRATCH_MNT/foo) +if [ $linkcount != 2 ]; then + echo "Unexpected link count after after snapshot delete: $linkcount expected: 2" +fi +echo "Silence is golden" + +# success, all done +status=0 +exit diff --git a/tests/btrfs/218.out b/tests/btrfs/218.out new file mode 100644 index 000000000000..1ef372a2a24b --- /dev/null +++ b/tests/btrfs/218.out @@ -0,0 +1,2 @@ +QA output created by 218 +Silence is golden diff --git a/tests/btrfs/group b/tests/btrfs/group index 8dcb23156236..94c1ec2806dc 100644 --- a/tests/btrfs/group +++ b/tests/btrfs/group @@ -220,3 +220,4 @@ 215 auto quick 216 auto quick seed 217 auto quick trim dangerous +218 auto quick subvol -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 1/1] btrfs: Track subdirectories in nlink 2020-08-27 15:04 [PATCH 0/1 NOT FOR MERGE] Basic subdir tracking in nlink Nikolay Borisov 2020-08-27 15:04 ` [PATCH] btrfs/218: Test for i_nlink tracking Nikolay Borisov @ 2020-08-27 15:04 ` Nikolay Borisov 2020-08-31 13:27 ` David Sterba 2020-08-27 15:04 ` [PATCH] btrfs-progs: check: Support precise nlink tracking Nikolay Borisov 2 siblings, 1 reply; 5+ messages in thread From: Nikolay Borisov @ 2020-08-27 15:04 UTC (permalink / raw) To: linux-btrfs; +Cc: Nikolay Borisov This adds the necessary calls to inc_nlink so that the number of subdirectories are accounted for in the i_nlink count of a directory. It works also for subvolumes and snapshots. Unfortunately the state of the on-disk i_nlink is codified in the tree checker so I had to remove the check but such filesystems will refuse to mount on older kernels. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/inode.c | 13 +++++++++++-- fs/btrfs/ioctl.c | 10 +++++++--- fs/btrfs/transaction.c | 12 ++++++++---- fs/btrfs/tree-checker.c | 7 +------ 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 40fed3285f62..e82eb501fe0d 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -3635,6 +3635,8 @@ static int __btrfs_unlink_inode(struct btrfs_trans_handle *trans, inode_inc_iversion(&dir->vfs_inode); inode->vfs_inode.i_ctime = dir->vfs_inode.i_mtime = dir->vfs_inode.i_ctime = current_time(&inode->vfs_inode); + if (S_ISDIR(inode->vfs_inode.i_mode)) + drop_nlink(&dir->vfs_inode); ret = btrfs_update_inode(trans, root, &dir->vfs_inode); out: return ret; @@ -3798,9 +3800,12 @@ static int btrfs_unlink_subvol(struct btrfs_trans_handle *trans, btrfs_i_size_write(BTRFS_I(dir), dir->i_size - name_len * 2); inode_inc_iversion(dir); dir->i_mtime = dir->i_ctime = current_time(dir); + drop_nlink(dir); ret = btrfs_update_inode_fallback(trans, root, dir); - if (ret) + if (ret) { btrfs_abort_transaction(trans, ret); + inc_nlink(dir); + } out: btrfs_free_path(path); return ret; @@ -6137,9 +6142,13 @@ int btrfs_add_link(struct btrfs_trans_handle *trans, parent_inode->vfs_inode.i_mtime = now; parent_inode->vfs_inode.i_ctime = now; } + if (S_ISDIR(inode->vfs_inode.i_mode)) + inc_nlink(&parent_inode->vfs_inode); ret = btrfs_update_inode(trans, root, &parent_inode->vfs_inode); - if (ret) + if (ret) { btrfs_abort_transaction(trans, ret); + drop_nlink(&parent_inode->vfs_inode); + } return ret; fail_dir_item: diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index f12c3df3c216..96b8daa8e8e1 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -716,21 +716,22 @@ static noinline int create_subvol(struct inode *dir, BTRFS_FT_DIR, index); if (ret) { btrfs_abort_transaction(trans, ret); - goto fail; + goto fail_drop_nlink; } btrfs_i_size_write(BTRFS_I(dir), dir->i_size + namelen * 2); + inc_nlink(dir); ret = btrfs_update_inode(trans, root, dir); if (ret) { btrfs_abort_transaction(trans, ret); - goto fail; + goto fail_drop_nlink; } ret = btrfs_add_root_ref(trans, objectid, root->root_key.objectid, btrfs_ino(BTRFS_I(dir)), index, name, namelen); if (ret) { btrfs_abort_transaction(trans, ret); - goto fail; + goto fail_drop_nlink; } ret = btrfs_uuid_tree_add(trans, root_item->uuid, @@ -738,6 +739,9 @@ static noinline int create_subvol(struct inode *dir, if (ret) btrfs_abort_transaction(trans, ret); +fail_drop_nlink: + if (ret) + drop_nlink(dir); fail: kfree(root_item); trans->block_rsv = NULL; diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index a1a8a35263a9..7717c4f522aa 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -1677,17 +1677,18 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans, dentry->d_name.len * 2); parent_inode->i_mtime = parent_inode->i_ctime = current_time(parent_inode); + inc_nlink(parent_inode); ret = btrfs_update_inode_fallback(trans, parent_root, parent_inode); if (ret) { btrfs_abort_transaction(trans, ret); - goto fail; + goto fail_drop_nlink; } ret = btrfs_uuid_tree_add(trans, new_root_item->uuid, BTRFS_UUID_KEY_SUBVOL, objectid); if (ret) { btrfs_abort_transaction(trans, ret); - goto fail; + goto fail_drop_nlink; } if (!btrfs_is_empty_uuid(new_root_item->received_uuid)) { ret = btrfs_uuid_tree_add(trans, new_root_item->received_uuid, @@ -1695,16 +1696,19 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans, objectid); if (ret && ret != -EEXIST) { btrfs_abort_transaction(trans, ret); - goto fail; + goto fail_drop_nlink; } } ret = btrfs_run_delayed_refs(trans, (unsigned long)-1); if (ret) { btrfs_abort_transaction(trans, ret); - goto fail; + goto fail_drop_nlink; } +fail_drop_nlink: + if (ret) + drop_nlink(parent_inode); fail: pending->error = ret; dir_item_existed: diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c index 517b44300a05..c353700b03c5 100644 --- a/fs/btrfs/tree-checker.c +++ b/fs/btrfs/tree-checker.c @@ -1015,12 +1015,7 @@ static int check_inode_item(struct extent_buffer *leaf, return -EUCLEAN; } } - if (S_ISDIR(mode) && btrfs_inode_nlink(leaf, iitem) > 1) { - inode_item_err(leaf, slot, - "invalid nlink: has %u expect no more than 1 for dir", - btrfs_inode_nlink(leaf, iitem)); - return -EUCLEAN; - } + if (btrfs_inode_flags(leaf, iitem) & ~BTRFS_INODE_FLAG_MASK) { inode_item_err(leaf, slot, "unknown flags detected: 0x%llx", -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] btrfs: Track subdirectories in nlink 2020-08-27 15:04 ` [PATCH 1/1] btrfs: Track subdirectories in nlink Nikolay Borisov @ 2020-08-31 13:27 ` David Sterba 0 siblings, 0 replies; 5+ messages in thread From: David Sterba @ 2020-08-31 13:27 UTC (permalink / raw) To: Nikolay Borisov; +Cc: linux-btrfs On Thu, Aug 27, 2020 at 06:04:25PM +0300, Nikolay Borisov wrote: > This adds the necessary calls to inc_nlink so that the number of > subdirectories are accounted for in the i_nlink count of a directory. It > works also for subvolumes and snapshots. Unfortunately the state of the > on-disk i_nlink is codified in the tree checker so I had to remove the > check but such filesystems will refuse to mount on older kernels. Not refusing to mount but it would return EUCLEAN when accessing the directory and probably turning the fs to read-only, aborting any running transactions on the way. > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/inode.c | 13 +++++++++++-- > fs/btrfs/ioctl.c | 10 +++++++--- > fs/btrfs/transaction.c | 12 ++++++++---- > fs/btrfs/tree-checker.c | 7 +------ > 4 files changed, 27 insertions(+), 15 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 40fed3285f62..e82eb501fe0d 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -3635,6 +3635,8 @@ static int __btrfs_unlink_inode(struct btrfs_trans_handle *trans, > inode_inc_iversion(&dir->vfs_inode); > inode->vfs_inode.i_ctime = dir->vfs_inode.i_mtime = > dir->vfs_inode.i_ctime = current_time(&inode->vfs_inode); > + if (S_ISDIR(inode->vfs_inode.i_mode)) > + drop_nlink(&dir->vfs_inode); This should not be unconditional, only decrease links if it's not 1, similarly the inc_nlink calls. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] btrfs-progs: check: Support precise nlink tracking 2020-08-27 15:04 [PATCH 0/1 NOT FOR MERGE] Basic subdir tracking in nlink Nikolay Borisov 2020-08-27 15:04 ` [PATCH] btrfs/218: Test for i_nlink tracking Nikolay Borisov 2020-08-27 15:04 ` [PATCH 1/1] btrfs: Track subdirectories in nlink Nikolay Borisov @ 2020-08-27 15:04 ` Nikolay Borisov 2 siblings, 0 replies; 5+ messages in thread From: Nikolay Borisov @ 2020-08-27 15:04 UTC (permalink / raw) To: linux-btrfs; +Cc: Nikolay Borisov This commit makes progs compatible with a kernel that has implemented subdirectory tracking in nlink. To achieve this the logic is modified such that found_link is incremented for the parent dir for every DIR_ITEM found which points to a subdirectory. Another change is to always set found_link to 1 when parsing INODE_ITEM which corresponds to a subvolume/snapshot root. This is to account for the fact that such inodes are the root in their respective fs trees so they won't have their found_link count bumped when parsing their DIR_ITEM in the parent directory. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- check/main.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/check/main.c b/check/main.c index c622caa8e04f..eec18132f8e5 100644 --- a/check/main.c +++ b/check/main.c @@ -908,6 +908,8 @@ static int process_inode_item(struct extent_buffer *eb, if (S_ISLNK(rec->imode) && flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND)) rec->errors |= I_ERR_ODD_INODE_FLAGS; + if (S_ISDIR(rec->imode) && rec->ino == BTRFS_FIRST_FREE_OBJECTID) + rec->found_link = 1; /* * We don't have accurate root info to determine the correct * inode generation uplimit, use super_generation + 1 anyway @@ -1421,6 +1423,16 @@ static int process_dir_item(struct extent_buffer *eb, goto next; } + if ((location.type == BTRFS_INODE_ITEM_KEY || + location.type == BTRFS_ROOT_ITEM_KEY) && filetype == BTRFS_FT_DIR && + key->type == BTRFS_DIR_ITEM_KEY) { + struct inode_record *dir_rec = get_inode_rec(inode_cache, + key->objectid, 1); + BUG_ON(IS_ERR(rec)); + dir_rec->found_link++; + maybe_free_inode_rec(inode_cache, dir_rec); + } + if (location.type == BTRFS_INODE_ITEM_KEY) { add_inode_backref(inode_cache, location.objectid, key->objectid, key->offset, namebuf, @@ -1893,7 +1905,7 @@ static int check_root_dir(struct inode_record *rec) goto out; } - if (rec->nlink != 1 || rec->found_link != 0) { + if (rec->nlink != rec->found_link) { rec->errors |= I_ERR_LINK_COUNT_WRONG; goto out; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-31 13:33 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-27 15:04 [PATCH 0/1 NOT FOR MERGE] Basic subdir tracking in nlink Nikolay Borisov 2020-08-27 15:04 ` [PATCH] btrfs/218: Test for i_nlink tracking Nikolay Borisov 2020-08-27 15:04 ` [PATCH 1/1] btrfs: Track subdirectories in nlink Nikolay Borisov 2020-08-31 13:27 ` David Sterba 2020-08-27 15:04 ` [PATCH] btrfs-progs: check: Support precise nlink tracking Nikolay Borisov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).