Remove projectquota space upgrade and usage update of ZFS_INVALID_PROJID

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 <projid> -s
<file/dir>`, 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 <jitendra.patidar@nutanix.com>

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);
 

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

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.
+# Check whether zpool 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.
 #
 # STRATEGY:
-# 1. Create a pool with all features disabled
+# 1. Create a pool with projectquota features disabled
-# 2. Create a few dataset for testing
+# 2. Create a dataset for testing
-# 3. Make sure automatic upgrade work
+# 3. Validate projectquota upgrade works
-# 4. Make sure manual upgrade work
 #
 
 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 zpool create -o feature@project_quota=disabled -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 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
+# pool upgrade should enable project quota
-zfs projectspace -o used $TESTPOOL | grep -q "USED" &&
+zpool get feature@project_quota -ovalue -H $TESTPOOL | grep -q "enabled" ||
-	log_fail "project quota should be disabled after pool upgrade"
+	log_fail "project quota should be enabled after pool upgrade"
 
-# Mount dataset should trigger upgrade
+# set projectquota should succeed after upgrade
-log_must zfs mount $TESTPOOL/fs1
+log_must zfs set projectquota@100=100m $TESTPOOL/fs3
-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"
 
-# Create file should trigger dataset upgrade
+# set projectobjquota should succeed after upgrade
-log_must mkfile 1m $TESTDIR/fs2/dir/tf
+log_must zfs set projectobjquota@100=1000 $TESTPOOL/fs3
-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"
 
-# "lsattr -p" should NOT trigger upgrade
+# 'chattr -p' should succeed after 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
 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" ||
+# 'chattr +P' should succeed after upgrade
-	log_fail "project quota should be enabled for $TESTPOOL/fs3"
+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"