Web lists-archives.com

[PATCH/RFC] staging/lustre: Rework class_process_proc_param




Ever since sysfs migration, class_process_proc_param stopped working
correctly as all the useful params were no longer present as lvars.
Replace all the nasty fake proc writes with hopefully less nasty
kobject attribute search and then update the attributes as needed.

Signed-off-by: Oleg Drokin <green@xxxxxxxxxxxxxx>
Reported-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
---
Al has quite rightfully complained in the past that class_process_proc_param
is a terrible piece of code and needs to go.
This patch is an attempt at improving it somewhat and in process drop
all the user/kernel address space games we needed to play to make it work
in the past (and which I suspect attracted Al's attention in the first place).

Now I wonder if iterating kobject attributes like that would be ok with
you Greg, or do you think there is a better way?
class_find_write_attr could be turned into something generic since it's
certainly convenient to reuse same table of name-write_method pairs,
but I did some cursory research and nobody else seems to need anything
of the sort in-tree.

I know ll_process_config is still awful and I will likely just
replace the current hack with kset_find_obj, but I just wanted to make
sure this new approach would be ok before spending too much time on it.

Thanks!

 drivers/staging/lustre/lustre/include/obd_class.h  |  4 +-
 drivers/staging/lustre/lustre/llite/llite_lib.c    | 10 +--
 drivers/staging/lustre/lustre/lov/lov_obd.c        |  3 +-
 drivers/staging/lustre/lustre/mdc/mdc_request.c    |  3 +-
 .../staging/lustre/lustre/obdclass/obd_config.c    | 78 ++++++++++------------
 drivers/staging/lustre/lustre/osc/osc_request.c    |  3 +-
 6 files changed, 44 insertions(+), 57 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/obd_class.h b/drivers/staging/lustre/lustre/include/obd_class.h
index 083a6ff..badafb8 100644
--- a/drivers/staging/lustre/lustre/include/obd_class.h
+++ b/drivers/staging/lustre/lustre/include/obd_class.h
@@ -114,8 +114,8 @@ typedef int (*llog_cb_t)(const struct lu_env *, struct llog_handle *,
 			 struct llog_rec_hdr *, void *);
 /* obd_config.c */
 int class_process_config(struct lustre_cfg *lcfg);
-int class_process_proc_param(char *prefix, struct lprocfs_vars *lvars,
-			     struct lustre_cfg *lcfg, void *data);
+int class_process_attr_param(char *prefix, struct kobject *kobj,
+			     struct lustre_cfg *lcfg);
 struct obd_device *class_incref(struct obd_device *obd,
 				const char *scope, const void *source);
 void class_decref(struct obd_device *obd,
diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
index 7b80040..192b877 100644
--- a/drivers/staging/lustre/lustre/llite/llite_lib.c
+++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
@@ -2259,7 +2259,7 @@ int ll_obd_statfs(struct inode *inode, void __user *arg)
 int ll_process_config(struct lustre_cfg *lcfg)
 {
 	char *ptr;
-	void *sb;
+	struct super_block *sb;
 	struct lprocfs_static_vars lvars;
 	unsigned long x;
 	int rc = 0;
@@ -2273,15 +2273,15 @@ int ll_process_config(struct lustre_cfg *lcfg)
 	rc = kstrtoul(ptr, 16, &x);
 	if (rc != 0)
 		return -EINVAL;
-	sb = (void *)x;
+	sb = (struct super_block *)x;
 	/* This better be a real Lustre superblock! */
-	LASSERT(s2lsi((struct super_block *)sb)->lsi_lmd->lmd_magic == LMD_MAGIC);
+	LASSERT(s2lsi(sb)->lsi_lmd->lmd_magic == LMD_MAGIC);
 
 	/* Note we have not called client_common_fill_super yet, so
 	 * proc fns must be able to handle that!
 	 */
-	rc = class_process_proc_param(PARAM_LLITE, lvars.obd_vars,
-				      lcfg, sb);
+	rc = class_process_attr_param(PARAM_LLITE, &ll_s2sbi(sb)->ll_kobj,
+				      lcfg);
 	if (rc > 0)
 		rc = 0;
 	return rc;
diff --git a/drivers/staging/lustre/lustre/lov/lov_obd.c b/drivers/staging/lustre/lustre/lov/lov_obd.c
index b3161fb..c33a327 100644
--- a/drivers/staging/lustre/lustre/lov/lov_obd.c
+++ b/drivers/staging/lustre/lustre/lov/lov_obd.c
@@ -926,8 +926,7 @@ int lov_process_config_base(struct obd_device *obd, struct lustre_cfg *lcfg,
 
 		lprocfs_lov_init_vars(&lvars);
 
-		rc = class_process_proc_param(PARAM_LOV, lvars.obd_vars,
-					      lcfg, obd);
+		rc = class_process_attr_param(PARAM_LOV, &obd->obd_kobj, lcfg);
 		if (rc > 0)
 			rc = 0;
 		goto out;
diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
index 6bc2fb8..00387b8 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
@@ -2670,8 +2670,7 @@ static int mdc_process_config(struct obd_device *obd, u32 len, void *buf)
 	lprocfs_mdc_init_vars(&lvars);
 	switch (lcfg->lcfg_command) {
 	default:
-		rc = class_process_proc_param(PARAM_MDC, lvars.obd_vars,
-					      lcfg, obd);
+		rc = class_process_attr_param(PARAM_MDC, &obd->obd_kobj, lcfg);
 		if (rc > 0)
 			rc = 0;
 		break;
diff --git a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c
index 8fce88f..08fd126 100644
--- a/drivers/staging/lustre/lustre/obdclass/obd_config.c
+++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c
@@ -995,26 +995,42 @@ int class_process_config(struct lustre_cfg *lcfg)
 }
 EXPORT_SYMBOL(class_process_config);
 
-int class_process_proc_param(char *prefix, struct lprocfs_vars *lvars,
-			     struct lustre_cfg *lcfg, void *data)
+static int class_find_write_attr(struct kobject *kobj, char *name, int namelen,
+				 char *val, int vallen)
+{
+	struct attribute *attr;
+	struct kobj_type *kt = get_ktype(kobj);
+	int i;
+
+	/* Empty object? */
+	if (!kt || !kt->default_attrs)
+		return -ENOENT;
+
+	for (i = 0; (attr = kt->default_attrs[i]) != NULL; i++) {
+		if (!strncmp(attr->name, name, namelen) &&
+		    namelen == strlen(attr->name)) {
+			if (kt->sysfs_ops && kt->sysfs_ops->store)
+				return kt->sysfs_ops->store(kobj, attr, val,
+							    vallen);
+			return -EINVAL;
+		}
+	}
+
+	return -ENOENT;
+}
+
+int class_process_attr_param(char *prefix, struct kobject *kobj,
+			     struct lustre_cfg *lcfg)
 {
-	struct lprocfs_vars *var;
-	struct file fakefile;
-	struct seq_file fake_seqfile;
 	char *key, *sval;
 	int i, keylen, vallen;
-	int matched = 0, j = 0;
 	int rc = 0;
-	int skip = 0;
 
 	if (lcfg->lcfg_command != LCFG_PARAM) {
 		CERROR("Unknown command: %d\n", lcfg->lcfg_command);
 		return -EINVAL;
 	}
 
-	/* fake a seq file so that var->fops->write can work... */
-	fakefile.private_data = &fake_seqfile;
-	fake_seqfile.private = data;
 	/* e.g. tunefs.lustre --param mdt.group_upcall=foo /r/tmp/lustre-mdt
 	 * or   lctl conf_param lustre-MDT0000.mdt.group_upcall=bar
 	 * or   lctl conf_param lustre-OST0000.osc.max_dirty_mb=36
@@ -1038,39 +1054,16 @@ int class_process_proc_param(char *prefix, struct lprocfs_vars *lvars,
 		keylen = sval - key;
 		sval++;
 		vallen = strlen(sval);
-		matched = 0;
-		j = 0;
-		/* Search proc entries */
-		while (lvars[j].name) {
-			var = &lvars[j];
-			if (!class_match_param(key, var->name, NULL) &&
-			    keylen == strlen(var->name)) {
-				matched++;
-				rc = -EROFS;
-				if (var->fops && var->fops->write) {
-					mm_segment_t oldfs;
-
-					oldfs = get_fs();
-					set_fs(KERNEL_DS);
-					rc = var->fops->write(&fakefile,
-						(const char __user *)sval,
-								vallen, NULL);
-					set_fs(oldfs);
-				}
-				break;
-			}
-			j++;
-		}
-		if (!matched) {
+		rc = class_find_write_attr(kobj, key, keylen, sval, vallen);
+
+		if (rc == -ENOENT) {
 			CERROR("%.*s: %s unknown param %s\n",
 			       (int)strlen(prefix) - 1, prefix,
 			       (char *)lustre_cfg_string(lcfg, 0), key);
 			/* rc = -EINVAL;	continue parsing other params */
-			skip++;
 		} else if (rc < 0) {
-			CERROR("%s: error writing proc entry '%s': rc = %d\n",
-			       prefix, var->name, rc);
-			rc = 0;
+			CERROR("%s: error writing proc entry '%.*s': rc = %d\n",
+			       prefix, keylen, key, rc);
 		} else {
 			CDEBUG(D_CONFIG, "%s.%.*s: Set parameter %.*s=%s\n",
 			       lustre_cfg_string(lcfg, 0),
@@ -1079,13 +1072,10 @@ int class_process_proc_param(char *prefix, struct lprocfs_vars *lvars,
 		}
 	}
 
-	if (rc > 0)
-		rc = 0;
-	if (!rc && skip)
-		rc = skip;
-	return rc;
+	/* Even if we did not find anything to write to, keep processing */
+	return 0;
 }
-EXPORT_SYMBOL(class_process_proc_param);
+EXPORT_SYMBOL(class_process_attr_param);
 
 /** Parse a configuration llog, doing various manipulations on them
  * for various reasons, (modifications for compatibility, skip obsolete
diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
index d8aa3fb..158fb98 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -2773,8 +2773,7 @@ int osc_process_config_base(struct obd_device *obd, struct lustre_cfg *lcfg)
 
 	switch (lcfg->lcfg_command) {
 	default:
-		rc = class_process_proc_param(PARAM_OSC, lvars.obd_vars,
-					      lcfg, obd);
+		rc = class_process_attr_param(PARAM_OSC, &obd->obd_kobj, lcfg);
 		if (rc > 0)
 			rc = 0;
 		break;
-- 
2.9.3