From 0035e10f098574c13c9a07c1cd93e640279666ce Mon Sep 17 00:00:00 2001 From: Jitendra Patidar Date: Thu, 22 Aug 2024 11:26:27 +0000 Subject: [PATCH] Remove projectquota space upgrade and usage update of ZFS_DEFAULT_PROJID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upgrading to feature@project_quota, currently trigger upgrade task around project quota usage upgrade, which marks each inode dirty via, dmu_objset_id_quota_upgrade()->dmu_objset_id_quota_upgrade_cb()-> dmu_objset_space_upgrade(). Project quota space upgrade task marks each dnode dirty, expecting that when the dnode_sync() is done, dnode’s project usage would be accounted in projid=0 (ZFS_DEFAULT_PROJID). But as there is no change in projid, so effectively, project quota upgrade task doesn't change anything. When actual projid is set on an object via `zfs project -p -s `, then object usage gets account-ed under the projid set. But usage for projid=0 (ZFS_DEFAULT_PROJID) underflows and becomes negative. Because quota update task moves usage from projid "0" to new projid set. Solution: dmu_objset_space_upgrade() for projectquota doesn't change the usage accounting, so skip it. This effectively avoids dirtying large number of dnodes, which is un-necessary. Usage accounting for projid=0 (ZFS_DEFAULT_PROJID) doesn't give any real benefit and it can't be updated for objects created before project_quota feature upgrade, so skip updating usage for ZFS_DEFAULT_PROJID in do_*quota_update() both for old and new objects. Effectively no usage accounting for ZFS_DEFAULT_PROJID. Signed-off-by: Jitendra Patidar --- module/zfs/dmu_objset.c | 15 +++- .../upgrade/upgrade_projectquota_001_pos.ksh | 82 +++++++------------ 2 files changed, 41 insertions(+), 56 deletions(-) diff --git a/module/zfs/dmu_objset.c b/module/zfs/dmu_objset.c index 8f00e6577bc5..9a168514423e 100644 --- a/module/zfs/dmu_objset.c +++ b/module/zfs/dmu_objset.c @@ -2035,7 +2035,8 @@ do_userquota_update(objset_t *os, userquota_cache_t *cache, uint64_t used, (void) snprintf(name, sizeof (name), "%llx", (longlong_t)group); userquota_update_cache(&cache->uqc_group_deltas, name, delta); - if (dmu_objset_projectquota_enabled(os)) { + if (dmu_objset_projectquota_enabled(os) && + project != ZFS_DEFAULT_PROJID) { (void) snprintf(name, sizeof (name), "%llx", (longlong_t)project); userquota_update_cache(&cache->uqc_project_deltas, @@ -2060,7 +2061,8 @@ do_userobjquota_update(objset_t *os, userquota_cache_t *cache, uint64_t flags, (longlong_t)group); userquota_update_cache(&cache->uqc_group_deltas, name, delta); - if (dmu_objset_projectquota_enabled(os)) { + if (dmu_objset_projectquota_enabled(os) && + project != ZFS_DEFAULT_PROJID) { (void) snprintf(name, sizeof (name), DMU_OBJACCT_PREFIX "%llx", (longlong_t)project); userquota_update_cache(&cache->uqc_project_deltas, @@ -2514,7 +2516,14 @@ dmu_objset_id_quota_upgrade_cb(objset_t *os) dmu_objset_ds(os)->ds_feature_activation[ SPA_FEATURE_PROJECT_QUOTA] = (void *)B_TRUE; - err = dmu_objset_space_upgrade(os); + /* + * By marking each inode dirty in dmu_objset_space_upgrade() for + * projectquota doesn't get usage accounted, as projid is not changed. + * Object usage gets accounted, when projid is set on it. So, space + * upgrade for the project quota is un-necessary. + */ + if (!dmu_objset_userobjspace_present(os)) + err = dmu_objset_space_upgrade(os); if (err) return (err); diff --git a/tests/zfs-tests/tests/functional/upgrade/upgrade_projectquota_001_pos.ksh b/tests/zfs-tests/tests/functional/upgrade/upgrade_projectquota_001_pos.ksh index 2c365e37af23..b9c17cb63eec 100755 --- a/tests/zfs-tests/tests/functional/upgrade/upgrade_projectquota_001_pos.ksh +++ b/tests/zfs-tests/tests/functional/upgrade/upgrade_projectquota_001_pos.ksh @@ -29,15 +29,12 @@ # # DESCRIPTION: # -# Check whether zfs upgrade for project quota works or not. -# The project quota is per dataset based feature, this test -# will create multiple datasets and try different upgrade methods. +# Check whether zpool upgrade for project quota works or not. # # STRATEGY: -# 1. Create a pool with all features disabled -# 2. Create a few dataset for testing -# 3. Make sure automatic upgrade work -# 4. Make sure manual upgrade work +# 1. Create a pool with projectquota features disabled +# 2. Create a dataset for testing +# 3. Validate projectquota upgrade works # verify_runnable "global" @@ -49,16 +46,7 @@ fi log_assert "pool upgrade for projectquota should work" log_onexit cleanup_upgrade -log_must zpool create -d -m $TESTDIR $TESTPOOL $TMPDEV - -log_must mkfiles $TESTDIR/tf $((RANDOM % 100 + 1)) -log_must zfs create $TESTPOOL/fs1 -log_must mkfiles $TESTDIR/fs1/tf $((RANDOM % 100 + 1)) -log_must zfs umount $TESTPOOL/fs1 - -log_must zfs create $TESTPOOL/fs2 -log_must mkdir $TESTDIR/fs2/dir -log_must mkfiles $TESTDIR/fs2/tf $((RANDOM % 100 + 1)) +log_must zpool create -o feature@project_quota=disabled -m $TESTDIR $TESTPOOL $TMPDEV log_must zfs create $TESTPOOL/fs3 log_must mkdir $TESTDIR/fs3/dir @@ -66,14 +54,14 @@ log_must mkfiles $TESTDIR/fs3/tf $((RANDOM % 100 + 1)) log_must set_xattr_stdin passwd $TESTDIR/fs3/dir < /etc/passwd # Make sure project quota is disabled -zfs projectspace -o used $TESTPOOL | grep -q "USED" && +zpool get feature@project_quota -ovalue -H $TESTPOOL | grep -q "disabled" || log_fail "project quota should be disabled initially" # set projectquota before upgrade will fail -log_mustnot zfs set projectquota@100=100m $TESTDIR/fs3 +log_mustnot zfs set projectquota@100=100m $TESTPOOL/fs3 # set projectobjquota before upgrade will fail -log_mustnot zfs set projectobjquota@100=1000 $TESTDIR/fs3 +log_mustnot zfs set projectobjquota@100=1000 $TESTPOOL/fs3 # 'chattr -p' should fail before upgrade log_mustnot chattr -p 100 $TESTDIR/fs3/dir @@ -83,33 +71,32 @@ log_mustnot chattr +P $TESTDIR/fs3/dir # Upgrade zpool to support all features log_must zpool upgrade $TESTPOOL +zpool get feature@project_quota -ovalue -H $TESTPOOL -# Double check project quota is disabled -zfs projectspace -o used $TESTPOOL | grep -q "USED" && - log_fail "project quota should be disabled after pool upgrade" +# pool upgrade should enable project quota +zpool get feature@project_quota -ovalue -H $TESTPOOL | grep -q "enabled" || + log_fail "project quota should be enabled after pool upgrade" -# Mount dataset should trigger upgrade -log_must zfs mount $TESTPOOL/fs1 -log_must sleep 3 # upgrade done in the background so let's wait for a while -zfs projectspace -o used $TESTPOOL/fs1 | grep -q "USED" || - log_fail "project quota should be enabled for $TESTPOOL/fs1" +# set projectquota should succeed after upgrade +log_must zfs set projectquota@100=100m $TESTPOOL/fs3 -# Create file should trigger dataset upgrade -log_must mkfile 1m $TESTDIR/fs2/dir/tf -log_must sleep 3 # upgrade done in the background so let's wait for a while -zfs projectspace -o used $TESTPOOL/fs2 | grep -q "USED" || - log_fail "project quota should be enabled for $TESTPOOL/fs2" +# set projectobjquota should succeed after upgrade +log_must zfs set projectobjquota@100=1000 $TESTPOOL/fs3 -# "lsattr -p" should NOT trigger upgrade -log_must lsattr -p -d $TESTDIR/fs3/dir -zfs projectspace -o used $TESTPOOL/fs3 | grep -q "USED" && - log_fail "project quota should not active for $TESTPOOL/fs3" - -# 'chattr -p' should trigger dataset upgrade +# 'chattr -p' should succeed after upgrade log_must chattr -p 100 $TESTDIR/fs3/dir -log_must sleep 5 # upgrade done in the background so let's wait for a while -zfs projectspace -o used $TESTPOOL/fs3 | grep -q "USED" || - log_fail "project quota should be enabled for $TESTPOOL/fs3" + +# 'chattr +P' should succeed after upgrade +log_must chattr +P $TESTDIR/fs3/dir + +# project quota should be active +zpool get feature@project_quota -ovalue -H $TESTPOOL | grep -q "active" || + log_fail "project quota should be active after chattr" +# project id 100 usage should be accounted +zfs projectspace -o name -H $TESTPOOL/fs3 | grep -q "100" || + log_fail "project id 100 usage should be accounted for $TESTPOOL/fs3" + +# xattr inodes should be accounted in project quota dirino=$(stat -c '%i' $TESTDIR/fs3/dir) log_must zdb -ddddd $TESTPOOL/fs3 $dirino xattrdirino=$(zdb -ddddd $TESTPOOL/fs3 $dirino |grep -w "xattr" |awk '{print $2}') @@ -129,15 +116,4 @@ cnt=$(get_prop projectobjused@100 $TESTPOOL/fs3) [[ $cnt -ne $expectedcnt ]] && log_fail "projectquota accounting failed $cnt" -# All in all, after having been through this, the dataset for testpool -# still shouldn't be upgraded -zfs projectspace -o used $TESTPOOL | grep -q "USED" && - log_fail "project quota should be disabled for $TESTPOOL" - -# Manual upgrade root dataset -# uses an ioctl which will wait for the upgrade to be done before returning -log_must zfs set version=current $TESTPOOL -zfs projectspace -o used $TESTPOOL | grep -q "USED" || - log_fail "project quota should be enabled for $TESTPOOL" - log_pass "Project Quota upgrade done"