From 0424c8ffaade6487eb269a3881915852d388d418 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_INVALID_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. 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. When object doesn't have projid set, means its ZFS_INVALID_PROJID, so change zpl_get_file_info() to consider projid as ZFS_INVALID_PROJID instead of ZFS_DEFAULT_PROJID=0. And skip updating usage for ZFS_INVALID_PROJID in do_*quota_update(). This effectively avoid the underflow of usage accounting on ZFS_DEFAULT_PROJID. Signed-off-by: Jitendra Patidar --- module/zfs/dmu_objset.c | 26 +++++- module/zfs/zfs_quota.c | 2 +- .../upgrade/upgrade_projectquota_001_pos.ksh | 82 +++++++------------ 3 files changed, 53 insertions(+), 57 deletions(-) diff --git a/module/zfs/dmu_objset.c b/module/zfs/dmu_objset.c index 8f4fefa4f4dd..d3daa5508a76 100644 --- a/module/zfs/dmu_objset.c +++ b/module/zfs/dmu_objset.c @@ -2029,7 +2029,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_INVALID_PROJID) { (void) snprintf(name, sizeof (name), "%llx", (longlong_t)project); userquota_update_cache(&cache->uqc_project_deltas, @@ -2054,7 +2055,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_INVALID_PROJID) { (void) snprintf(name, sizeof (name), DMU_OBJACCT_PREFIX "%llx", (longlong_t)project); userquota_update_cache(&cache->uqc_project_deltas, @@ -2508,7 +2510,25 @@ 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)) { +#ifdef _KERNEL + char name[ZFS_MAX_DATASET_NAME_LEN] = {}; + dsl_dataset_name(os->os_dsl_dataset, name); + cmn_err(CE_NOTE, "%s %s[%d] Start objset space upgrade for %s", + __func__, current->comm, current->pid, name); +#endif + err = dmu_objset_space_upgrade(os); +#ifdef _KERNEL + cmn_err(CE_NOTE, "%s %s[%d] Done objset space upgrade for %s", + __func__, current->comm, current->pid, name); +#endif + } if (err) return (err); diff --git a/module/zfs/zfs_quota.c b/module/zfs/zfs_quota.c index 9b351eefc04e..7993dbe30884 100644 --- a/module/zfs/zfs_quota.c +++ b/module/zfs/zfs_quota.c @@ -47,7 +47,7 @@ zpl_get_file_info(dmu_object_type_t bonustype, const void *data, if (bonustype != DMU_OT_ZNODE && bonustype != DMU_OT_SA) return (SET_ERROR(ENOENT)); - zoi->zfi_project = ZFS_DEFAULT_PROJID; + zoi->zfi_project = ZFS_INVALID_PROJID; /* * If we have a NULL data pointer 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"