[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v8 09/12] xen: add runtime parameter access support to hypfs

On 14.05.20 17:02, Jan Beulich wrote:
On 14.05.2020 16:56, Jürgen Groß wrote:
On 14.05.20 14:10, Jan Beulich wrote:
On 14.05.2020 13:48, Jürgen Groß wrote:
On 14.05.20 12:20, Jan Beulich wrote:
On 08.05.2020 17:34, Juergen Gross wrote:
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -85,8 +85,43 @@ struct grant_table {
        struct grant_table_arch arch;
    -static int parse_gnttab_limit(const char *param, const char *arg,
-                              unsigned int *valp)
+unsigned int __read_mostly opt_max_grant_frames = 64;
+static unsigned int __read_mostly opt_max_maptrack_frames = 1024;
+#define GRANT_CUSTOM_VAL_SZ  12
+static char __read_mostly opt_max_grant_frames_val[GRANT_CUSTOM_VAL_SZ];
+static char __read_mostly opt_max_maptrack_frames_val[GRANT_CUSTOM_VAL_SZ];
+static void update_gnttab_par(struct param_hypfs *par, unsigned int val,
+                              char *parval)
+    snprintf(parval, GRANT_CUSTOM_VAL_SZ, "%u", val);
+    custom_runtime_set_var_sz(par, parval, GRANT_CUSTOM_VAL_SZ);
+static void __init gnttab_max_frames_init(struct param_hypfs *par)
+    update_gnttab_par(par, opt_max_grant_frames, opt_max_grant_frames_val);
+static void __init max_maptrack_frames_init(struct param_hypfs *par)
+    update_gnttab_par(par, opt_max_maptrack_frames,
+                      opt_max_maptrack_frames_val);
+#define opt_max_grant_frames_val    NULL
+#define opt_max_maptrack_frames_val NULL

This looks latently dangerous to me (in case new uses of these
two identifiers appeared), but I guess my alternative suggestion
will be at best controversial, too:

#define update_gnttab_par(par, val, unused) update_gnttab_par(par, val)
#define parse_gnttab_limit(par, arg, valp, unused) parse_gnttab_limit(par, arg, 

(placed right here)

Who else would use those identifiers not related to hypfs?

I can't see an obvious possible use, but people get creative, i.e.
you never know. Passing NULL into a function without it being
blindingly obvious that it won't ever get (de)referenced is an at
least theoretical risk imo.

Hmm, what about using a special type for those content variables?
Something like:

#define hypfs_string_var            char *
#define hypfs_string_var            char
#define opt_max_grant_frames_val    0
#define opt_max_maptrack_frames_val 0

And then use that as type for function parameters? This should make
dereferencing pretty hard.

Other than that I have no really good idea how to avoid this problem.

IOW (as suspected) you don't like my suggestion? Personally I think
yours, having more #define-s, is at least not better than mine.

Oh, maybe I misunderstood you here. I thought you weren't happy with
your solution either and suspected others (not me) wouldn't like it.

In case others don't reject it I'd be happy to use your variant.




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.