From 1cbc927ccb64e86e3e5273291f5309e9e082f7bb Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Thu, 28 Jul 2022 16:24:23 -0700 Subject: [PATCH] Only clear trying inode deletion bit when set try_delete_inode_items() is responsible for making sure that it's safe to delete an inode's persistent items. One of the things it has to check is that there isn't another deletion attempt on the inode in this mount. It sets a bit in lock data while it's working and backs off if the bit is already set. Unfortunately it was always clearing this bit as it exited, regardless of whether it set it or not. This would let the next attempt perform the deletion again before the working task had finished. This was often not a problem because background orphan scanning is the only source of regular concurrent deletion attempts. But it's a big problem if a deletion attempt takes a very long time. It gives enough time for an orphan scan attempt to clear the bit then try again and clobber on whoever is performing the very slow deletion. I hit this in a test that built files with an absurd number of fragmented extents. The second concurrent orphan attempt was able to proceed with deletion and performed a bunch of duplicate data extent frees and caused corruption. The fix is to only clear the bit if we set it. Now all concurrent attempts will back off until the first task is done. Signed-off-by: Zach Brown --- kmod/src/inode.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kmod/src/inode.c b/kmod/src/inode.c index 9d22f52a..62ef3093 100644 --- a/kmod/src/inode.c +++ b/kmod/src/inode.c @@ -1685,6 +1685,7 @@ static int try_delete_inode_items(struct super_block *sb, u64 ino) struct scoutfs_lock *lock = NULL; struct scoutfs_inode sinode; struct scoutfs_key key; + bool clear_trying = false; u64 group_nr; int bit_nr; int ret; @@ -1704,6 +1705,7 @@ static int try_delete_inode_items(struct super_block *sb, u64 ino) ret = 0; goto out; } + clear_trying = true; /* can't delete if it's cached in local or remote mounts */ if (scoutfs_omap_test(sb, ino) || test_bit_le(bit_nr, ldata->map.bits)) { @@ -1730,7 +1732,7 @@ static int try_delete_inode_items(struct super_block *sb, u64 ino) ret = delete_inode_items(sb, ino, &sinode, lock, orph_lock); out: - if (ldata) + if (clear_trying) clear_bit(bit_nr, ldata->trying); scoutfs_unlock(sb, lock, SCOUTFS_LOCK_WRITE);