ANDROID: Incremental fs: Make sysfs_name changeable on remount

Bug: 187829246
Test: incfs_test passes
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: I1762f170c8a8a2fb7672f65c402e82ab95aeef8a

fs/incfs/data_mgmt.c

@@ -75,7 +75,7 @@ struct mount_info *incfs_alloc_mount_info(struct super_block *sb,
 	INIT_DELAYED_WORK(&mi->mi_zstd_cleanup_work, zstd_free_workspace);
 	mutex_init(&mi->mi_le_mutex);
 
-	node = incfs_add_sysfs_node(options->sysfs_name);
+	node = incfs_add_sysfs_node(options->sysfs_name, mi);
 	if (IS_ERR(node)) {
 		error = PTR_ERR(node);
 		goto err;
@@ -130,13 +130,25 @@ int incfs_realloc_mount_info(struct mount_info *mi,
 		kfree(old_buffer);
 	}
 
-	if ((options->sysfs_name && !mi->mi_sysfs_node) ||
+	if (options->sysfs_name && !mi->mi_sysfs_node)
-	    (!options->sysfs_name && mi->mi_sysfs_node) ||
+		mi->mi_sysfs_node = incfs_add_sysfs_node(options->sysfs_name,
-	    (options->sysfs_name &&
+							 mi);
+	else if (!options->sysfs_name && mi->mi_sysfs_node) {
+		incfs_free_sysfs_node(mi->mi_sysfs_node);
+		mi->mi_sysfs_node = NULL;
+	} else if (options->sysfs_name &&
 		strcmp(options->sysfs_name,
-		       kobject_name(&mi->mi_sysfs_node->isn_sysfs_node)))) {
+		       kobject_name(&mi->mi_sysfs_node->isn_sysfs_node))) {
-		pr_err("incfs: Can't change sysfs_name mount option on remount\n");
+		incfs_free_sysfs_node(mi->mi_sysfs_node);
-		return -EOPNOTSUPP;
+		mi->mi_sysfs_node = incfs_add_sysfs_node(options->sysfs_name,
+							 mi);
+	}
+
+	if (IS_ERR(mi->mi_sysfs_node)) {
+		int err = PTR_ERR(mi->mi_sysfs_node);
+
+		mi->mi_sysfs_node = NULL;
+		return err;
 	}
 
 	mi->mi_options = *options;
@@ -1232,18 +1244,15 @@ static int wait_for_data_block(struct data_file *df, int block_index,
 	if (error)
 		return error;
 
-	if (!mi->mi_sysfs_node)
-		return 0;
-
 	if (delayed_pending) {
-		mi->mi_sysfs_node->isn_reads_delayed_pending++;
+		mi->mi_reads_delayed_pending++;
-		mi->mi_sysfs_node->isn_reads_delayed_pending_us +=
+		mi->mi_reads_delayed_pending_us +=
 			delayed_pending_us;
 	}
 
 	if (delayed_min_us) {
-		mi->mi_sysfs_node->isn_reads_delayed_min++;
+		mi->mi_reads_delayed_min++;
-		mi->mi_sysfs_node->isn_reads_delayed_min_us += delayed_min_us;
+		mi->mi_reads_delayed_min_us += delayed_min_us;
 	}
 
 	return 0;
@@ -1334,16 +1343,14 @@ ssize_t incfs_read_data_file_block(struct mem_range dst, struct file *f,
 		log_block_read(mi, &df->df_id, index);
 
 out:
-	if (mi->mi_sysfs_node) {
+	if (result == -ETIME)
-		if (result == -ETIME)
+		mi->mi_reads_failed_timed_out++;
-			mi->mi_sysfs_node->isn_reads_failed_timed_out++;
+	else if (result == -EBADMSG)
-		else if (result == -EBADMSG)
+		mi->mi_reads_failed_hash_verification++;
-			mi->mi_sysfs_node->isn_reads_failed_hash_verification++;
+	else if (result < 0)
-		else if (result < 0)
+		mi->mi_reads_failed_other++;
-			mi->mi_sysfs_node->isn_reads_failed_other++;
 
-		incfs_update_sysfs_error(f, index, result, mi, df);
+	incfs_update_sysfs_error(f, index, result, mi, df);
-	}
 
 	return result;
 }

fs/incfs/data_mgmt.h

@@ -199,6 +199,37 @@ struct mount_info {
 	u64		mi_le_time_us;
 	u32		mi_le_page;
 	u32		mi_le_errno;
+
+	/* Number of reads timed out */
+	u32 mi_reads_failed_timed_out;
+
+	/* Number of reads failed because hash verification failed */
+	u32 mi_reads_failed_hash_verification;
+
+	/* Number of reads failed for another reason */
+	u32 mi_reads_failed_other;
+
+	/* Number of reads delayed because page had to be fetched */
+	u32 mi_reads_delayed_pending;
+
+	/* Total time waiting for pages to be fetched */
+	u64 mi_reads_delayed_pending_us;
+
+	/*
+	 * Number of reads delayed because of per-uid min_time_us or
+	 * min_pending_time_us settings
+	 */
+	u32 mi_reads_delayed_min;
+
+	/* Total time waiting because of per-uid min_time_us or
+	 * min_pending_time_us settings.
+	 *
+	 * Note that if a read is initially delayed because we have to wait for
+	 * the page, then further delayed because of min_pending_time_us
+	 * setting, this counter gets incremented by only the further delay
+	 * time.
+	 */
+	u64 mi_reads_delayed_min_us;
 };
 
 struct data_file_block {

fs/incfs/sysfs.c

@@ -100,7 +100,7 @@ static ssize_t name##_show(struct kobject *kobj,			\
 	struct incfs_sysfs_node *node = container_of(kobj,		\
 			struct incfs_sysfs_node, isn_sysfs_node);	\
 									\
-	return sysfs_emit(buff, "%d\n", node->isn_##name);		\
+	return sysfs_emit(buff, "%d\n", node->isn_mi->mi_##name);	\
 }									\
 									\
 static struct kobj_attribute name##_attr = __ATTR_RO(name)
@@ -112,7 +112,7 @@ static ssize_t name##_show(struct kobject *kobj,			\
 	struct incfs_sysfs_node *node = container_of(kobj,		\
 			struct incfs_sysfs_node, isn_sysfs_node);	\
 									\
-	return sysfs_emit(buff, "%lld\n", node->isn_##name);		\
+	return sysfs_emit(buff, "%lld\n", node->isn_mi->mi_##name);	\
 }									\
 									\
 static struct kobj_attribute name##_attr = __ATTR_RO(name)
@@ -141,7 +141,7 @@ static void incfs_sysfs_release(struct kobject *kobj)
 	struct incfs_sysfs_node *node = container_of(kobj,
 				struct incfs_sysfs_node, isn_sysfs_node);
 
-	kfree(node);
+	complete(&node->isn_completion);
 }
 
 static const struct attribute_group mount_attr_group = {
@@ -153,7 +153,8 @@ static struct kobj_type incfs_kobj_node_ktype = {
 	.release	= &incfs_sysfs_release,
 };
 
-struct incfs_sysfs_node *incfs_add_sysfs_node(const char *name)
+struct incfs_sysfs_node *incfs_add_sysfs_node(const char *name,
+					      struct mount_info *mi)
 {
 	struct incfs_sysfs_node *node = NULL;
 	int error;
@@ -165,6 +166,9 @@ struct incfs_sysfs_node *incfs_add_sysfs_node(const char *name)
 	if (!node)
 		return ERR_PTR(-ENOMEM);
 
+	node->isn_mi = mi;
+
+	init_completion(&node->isn_completion);
 	kobject_init(&node->isn_sysfs_node, &incfs_kobj_node_ktype);
 	error = kobject_add(&node->isn_sysfs_node, instances_node, "%s", name);
 	if (error)
@@ -192,4 +196,6 @@ void incfs_free_sysfs_node(struct incfs_sysfs_node *node)
 
 	sysfs_remove_group(&node->isn_sysfs_node, &mount_attr_group);
 	kobject_put(&node->isn_sysfs_node);
+	wait_for_completion_interruptible(&node->isn_completion);
+	kfree(node);
 }

fs/incfs/sysfs.h

@@ -8,41 +8,15 @@
 struct incfs_sysfs_node {
 	struct kobject isn_sysfs_node;
 
-	/* Number of reads timed out */
+	struct completion isn_completion;
-	u32 isn_reads_failed_timed_out;
 
-	/* Number of reads failed because hash verification failed */
+	struct mount_info *isn_mi;
-	u32 isn_reads_failed_hash_verification;
-
-	/* Number of reads failed for another reason */
-	u32 isn_reads_failed_other;
-
-	/* Number of reads delayed because page had to be fetched */
-	u32 isn_reads_delayed_pending;
-
-	/* Total time waiting for pages to be fetched */
-	u64 isn_reads_delayed_pending_us;
-
-	/*
-	 * Number of reads delayed because of per-uid min_time_us or
-	 * min_pending_time_us settings
-	 */
-	u32 isn_reads_delayed_min;
-
-	/* Total time waiting because of per-uid min_time_us or
-	 * min_pending_time_us settings.
-	 *
-	 * Note that if a read is initially delayed because we have to wait for
-	 * the page, then further delayed because of min_pending_time_us
-	 * setting, this counter gets incremented by only the further delay
-	 * time.
-	 */
-	u64 isn_reads_delayed_min_us;
 };
 
 int incfs_init_sysfs(void);
 void incfs_cleanup_sysfs(void);
-struct incfs_sysfs_node *incfs_add_sysfs_node(const char *name);
+struct incfs_sysfs_node *incfs_add_sysfs_node(const char *name,
+					      struct mount_info *mi);
 void incfs_free_sysfs_node(struct incfs_sysfs_node *node);
 
 #endif

fs/incfs/vfs.c

@@ -23,6 +23,7 @@
 #include "format.h"
 #include "internal.h"
 #include "pseudo_files.h"
+#include "sysfs.h"
 #include "verity.h"
 
 static int incfs_remount_fs(struct super_block *sb, int *flags, char *data);
@@ -1895,5 +1896,9 @@ static int show_options(struct seq_file *m, struct dentry *root)
 	}
 	if (mi->mi_options.report_uid)
 		seq_puts(m, ",report_uid");
+
+	if (mi->mi_sysfs_node)
+		seq_printf(m, ",sysfs_name=%s",
+			   kobject_name(&mi->mi_sysfs_node->isn_sysfs_node));
 	return 0;
 }

tools/testing/selftests/filesystems/incfs/incfs_test.c

@@ -4527,6 +4527,88 @@ static int sysfs_test(const char *mount_dir)
 	return result;
 }
 
+static int sysfs_test_directories(bool one_present, bool two_present)
+{
+	int result = TEST_FAILURE;
+	struct stat st;
+
+	TESTEQUAL(stat("/sys/fs/incremental-fs/instances/1", &st),
+		  one_present ? 0 : -1);
+	if (one_present)
+		TESTCOND(S_ISDIR(st.st_mode));
+	else
+		TESTEQUAL(errno, ENOENT);
+	TESTEQUAL(stat("/sys/fs/incremental-fs/instances/2", &st),
+		  two_present ? 0 : -1);
+	if (two_present)
+		TESTCOND(S_ISDIR(st.st_mode));
+	else
+		TESTEQUAL(errno, ENOENT);
+
+	result = TEST_SUCCESS;
+out:
+	return result;
+}
+
+static int sysfs_rename_test(const char *mount_dir)
+{
+	int result = TEST_FAILURE;
+	char *backing_dir = NULL;
+	char *mount_dir2 = NULL;
+	int fd = -1;
+	char c;
+
+	/* Mount with no node */
+	TEST(backing_dir = create_backing_dir(mount_dir), backing_dir);
+	TESTEQUAL(mount_fs(mount_dir, backing_dir, 0), 0);
+	TESTEQUAL(sysfs_test_directories(false, false), 0);
+
+	/* Remount with node */
+	TESTEQUAL(mount_fs_opt(mount_dir, backing_dir, "sysfs_name=1", true),
+		  0);
+	TESTEQUAL(sysfs_test_directories(true, false), 0);
+	TEST(fd = open("/sys/fs/incremental-fs/instances/1/reads_delayed_min",
+		       O_RDONLY | O_CLOEXEC), fd != -1);
+	TESTEQUAL(pread(fd, &c, 1, 0), 1);
+	TESTEQUAL(c, '0');
+	TESTEQUAL(pread(fd, &c, 1, 0), 1);
+	TESTEQUAL(c, '0');
+
+	/* Rename node */
+	TESTEQUAL(mount_fs_opt(mount_dir, backing_dir, "sysfs_name=2", true),
+		  0);
+	TESTEQUAL(sysfs_test_directories(false, true), 0);
+	TESTEQUAL(pread(fd, &c, 1, 0), -1);
+
+	/* Try mounting another instance with same node name */
+	TEST(mount_dir2 = concat_file_name(backing_dir, "incfs-mount-dir2"),
+	     mount_dir2);
+	rmdir(mount_dir2); /* In case we crashed before */
+	TESTSYSCALL(mkdir(mount_dir2, 0777));
+	TEST(mount_fs_opt(mount_dir2, backing_dir, "sysfs_name=2", false),
+		  -1);
+
+	/* Try mounting another instance then remounting with existing name */
+	TESTEQUAL(mount_fs(mount_dir2, backing_dir, 0), 0);
+	TESTEQUAL(mount_fs_opt(mount_dir2, backing_dir, "sysfs_name=2", true),
+		  -1);
+
+	/* Remount with no node */
+	TESTEQUAL(mount_fs_opt(mount_dir, backing_dir, "", true),
+		  0);
+	TESTEQUAL(sysfs_test_directories(false, false), 0);
+
+	result = TEST_SUCCESS;
+out:
+	umount(mount_dir2);
+	rmdir(mount_dir2);
+	free(mount_dir2);
+	close(fd);
+	umount(mount_dir);
+	free(backing_dir);
+	return result;
+}
+
 static char *setup_mount_dir()
 {
 	struct stat st;
@@ -4647,6 +4729,7 @@ int main(int argc, char *argv[])
 		MAKE_TEST(truncate_test),
 		MAKE_TEST(stat_test),
 		MAKE_TEST(sysfs_test),
+		MAKE_TEST(sysfs_rename_test),
 	};
 #undef MAKE_TEST