cgroup/rdma: refactor resource parsing with match_table_t/match_token()

Replace the hand-rolled strsep/strcmp/match_string parsing in
rdmacg_resource_set_max() with a match_table_t and match_token()
pattern, following the convention used by user_proactive_reclaim()
and ioc_cost_model_write().

The old strncmp(value, RDMACG_MAX_STR, strlen(value)) also had two
bugs that are fixed by this refactor:

  - It matched "ma" as "max" because strncmp only compared the
    shorter strlen(value) bytes.

  - It silently accepted "hca_handle=" (empty value) as "max"
    because strncmp with n=0 always returns 0.

The match_token() approach also robustly handles extra whitespace in
the input by splitting on " \t\n" and skipping empty tokens.

Suggested-by: "Michal Koutný" <mkoutny@suse.com>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Tejun Heo <tj@kernel.org>

diff --git a/kernel/cgroup/rdma.c b/kernel/cgroup/rdma.c
index 4fdab4cf49e0f2b47109309fdd46df9b6e22d1dd..3df7c38ce4815abc353cb5a24cfea8011d5c9e6b 100644 (file)
--- a/kernel/cgroup/rdma.c
+++ b/kernel/cgroup/rdma.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/limits.h>
 #include <linux/slab.h>
 #include <linux/seq_file.h>
 #include <linux/cgroup.h>
@@ -17,6 +18,22 @@
 
 #define RDMACG_MAX_STR "max"
 
+enum rdmacg_limit_tokens {
+       RDMACG_HCA_HANDLE_VAL,
+       RDMACG_HCA_HANDLE_MAX,
+       RDMACG_HCA_OBJECT_VAL,
+       RDMACG_HCA_OBJECT_MAX,
+       NR_RDMACG_LIMIT_TOKENS,
+};
+
+static const match_table_t rdmacg_limit_tokens = {
+       { RDMACG_HCA_HANDLE_VAL,        "hca_handle=%d" },
+       { RDMACG_HCA_HANDLE_MAX,        "hca_handle=max"        },
+       { RDMACG_HCA_OBJECT_VAL,        "hca_object=%d" },
+       { RDMACG_HCA_OBJECT_MAX,        "hca_object=max"        },
+       { NR_RDMACG_LIMIT_TOKENS,       NULL                    },
+};
+
 /*
  * Protects list of resource pools maintained on per cgroup basis
  * and rdma device list.
@@ -355,62 +372,6 @@ void rdmacg_unregister_device(struct rdmacg_device *device)
 }
 EXPORT_SYMBOL(rdmacg_unregister_device);
 
-static int parse_resource(char *c, int *intval)
-{
-       substring_t argstr;
-       char *name, *value = c;
-       size_t len;
-       int ret, i;
-
-       name = strsep(&value, "=");
-       if (!name || !value)
-               return -EINVAL;
-
-       i = match_string(rdmacg_resource_names, RDMACG_RESOURCE_MAX, name);
-       if (i < 0)
-               return i;
-
-       len = strlen(value);
-
-       argstr.from = value;
-       argstr.to = value + len;
-
-       ret = match_int(&argstr, intval);
-       if (ret >= 0) {
-               if (*intval < 0)
-                       return -EINVAL;
-               return i;
-       }
-       if (strncmp(value, RDMACG_MAX_STR, len) == 0) {
-               *intval = S32_MAX;
-               return i;
-       }
-       return -EINVAL;
-}
-
-static int rdmacg_parse_limits(char *options,
-                              int *new_limits, unsigned long *enables)
-{
-       char *c;
-       int err = -EINVAL;
-
-       /* parse resource options */
-       while ((c = strsep(&options, " ")) != NULL) {
-               int index, intval;
-
-               index = parse_resource(c, &intval);
-               if (index < 0)
-                       goto err;
-
-               new_limits[index] = intval;
-               *enables |= BIT(index);
-       }
-       return 0;
-
-err:
-       return err;
-}
-
 static struct rdmacg_device *rdmacg_get_device_locked(const char *name)
 {
        struct rdmacg_device *device;
@@ -432,6 +393,7 @@ static ssize_t rdmacg_resource_set_max(struct kernfs_open_file *of,
        struct rdmacg_resource_pool *rpool;
        struct rdmacg_device *device;
        char *options = strstrip(buf);
+       char *p;
        int *new_limits;
        unsigned long enables = 0;
        int i = 0, ret = 0;
@@ -449,9 +411,45 @@ static ssize_t rdmacg_resource_set_max(struct kernfs_open_file *of,
                goto err;
        }
 
-       ret = rdmacg_parse_limits(options, new_limits, &enables);
-       if (ret)
-               goto parse_err;
+       /* parse resource limit tokens */
+       while ((p = strsep(&options, " \t\n"))) {
+               substring_t args[MAX_OPT_ARGS];
+               int tok, intval;
+
+               if (!*p)
+                       continue;
+
+               tok = match_token(p, rdmacg_limit_tokens, args);
+               switch (tok) {
+               case RDMACG_HCA_HANDLE_VAL:
+                       if (match_int(&args[0], &intval) || intval < 0) {
+                               ret = -EINVAL;
+                               goto parse_err;
+                       }
+                       new_limits[RDMACG_RESOURCE_HCA_HANDLE] = intval;
+                       enables |= BIT(RDMACG_RESOURCE_HCA_HANDLE);
+                       break;
+               case RDMACG_HCA_HANDLE_MAX:
+                       new_limits[RDMACG_RESOURCE_HCA_HANDLE] = S32_MAX;
+                       enables |= BIT(RDMACG_RESOURCE_HCA_HANDLE);
+                       break;
+               case RDMACG_HCA_OBJECT_VAL:
+                       if (match_int(&args[0], &intval) || intval < 0) {
+                               ret = -EINVAL;
+                               goto parse_err;
+                       }
+                       new_limits[RDMACG_RESOURCE_HCA_OBJECT] = intval;
+                       enables |= BIT(RDMACG_RESOURCE_HCA_OBJECT);
+                       break;
+               case RDMACG_HCA_OBJECT_MAX:
+                       new_limits[RDMACG_RESOURCE_HCA_OBJECT] = S32_MAX;
+                       enables |= BIT(RDMACG_RESOURCE_HCA_OBJECT);
+                       break;
+               default:
+                       ret = -EINVAL;
+                       goto parse_err;
+               }
+       }
 
        /* acquire lock to synchronize with hot plug devices */
        mutex_lock(&rdmacg_mutex);