| Message ID | 20220720153233.144129-17-jim.cromie@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DYNDBG: opt-in class'd debug for modules, use in drm. | expand |
On 7/20/22 11:32, Jim Cromie wrote: > Add kernel_param_ops and callbacks to apply a class-map to a > sysfs-node, which then can control classes defined in that class-map. > This supports uses like: > > echo 0x3 > /sys/module/drm/parameters/debug > > IE add these: > > - int param_set_dyndbg_classes() > - int param_get_dyndbg_classes() > - struct kernel_param_ops param_ops_dyndbg_classes > > Following the model of kernel/params.c STANDARD_PARAM_DEFS, these are > non-static and exported. This might be unnecessary here. > > get/set use an augmented kernel_param; the arg refs a new struct > ddebug_classes_bitmap_param, initialized by DYNAMIC_DEBUG_CLASSBITS > macro, which contains: > > BITS: a pointer to the user module's ulong holding the bits/state. By > ref'g the client's bit-state _var, we coordinate with existing code > (such as drm_debug_enabled) which uses the _var, so it works > unchanged, even as the foundation is switched out underneath it.. > Using a ulong allows use of BIT() etc. > > FLAGS: dyndbg.flags toggled by changes to bitmap. Usually just "p". > > MAP: a pointer to struct ddebug_classes_map, which maps those > class-names to .class_ids 0..N that the module is using. This > class-map is declared & initialized by DEFINE_DYNDBG_CLASSMAP. > > map-type: add support here for DD_CLASS_DISJOINT, DD_CLASS_VERBOSE. > > These 2 class-types both expect an integer; _DISJOINT treats input > like a bit-vector (ala drm.debug), and sets each bit accordingly. > > _VERBOSE treats input like a bit-pos:N, then sets bits(0..N)=1, and > bits(N+1..max)=0. This applies (bit<N) semantics on top of disjoint > bits. > > cases DD_CLASS_SYMBOLIC, DD_CLASS_LEVELS are included for the complete > picture, with commented out call to a following commit. > > NOTES: > > this now includes SYMBOLIC/LEVELS support, too tedious to keep > separate thru all the tweaking. > > get-param undoes the bit-pos -> bitmap transform that set-param does > on VERBOSE inputs, this gives the read-what-was-written property. > > _VERBOSE is overlay on _DISJOINT: > > verbose-maps still need class-names, even though theyre not usable at > the sysfs interface (unlike with _SYMBOLIC/_LEVELS). > > - It must have a "V0" name, > something below "V1" to turn "V1" off. > __pr_debug_cls(V0,..) is printk, don't do that. > > - "class names" is required at the >control interface. > - relative levels are not enforced at >control > > IOW this is possible, and maybe confusing: > > echo class V3 +p > control > echo class V1 -p > control > > IMO thats ok, relative verbosity is an interface property. > > Signed-off-by: Jim Cromie <jim.cromie@gmail.com> > --- > . drop kp->mod->name as unneeded (build-dependent) <lkp> > --- > include/linux/dynamic_debug.h | 18 ++++ > lib/dynamic_debug.c | 193 ++++++++++++++++++++++++++++++++++ > 2 files changed, 211 insertions(+) > > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h > index f57076e02767..b50bdd5c8184 100644 > --- a/include/linux/dynamic_debug.h > +++ b/include/linux/dynamic_debug.h > @@ -113,6 +113,12 @@ struct ddebug_class_map { > #define NUM_TYPE_ARGS(eltype, ...) \ > (sizeof((eltype[]) {__VA_ARGS__}) / sizeof(eltype)) > > +struct ddebug_classes_bitmap_param { > + unsigned long *bits; > + char flags[8]; > + const struct ddebug_class_map *map; > +}; > + > #if defined(CONFIG_DYNAMIC_DEBUG_CORE) > > int ddebug_add_module(struct _ddebug *tab, unsigned int num_debugs, > @@ -274,6 +280,10 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > KERN_DEBUG, prefix_str, prefix_type, \ > rowsize, groupsize, buf, len, ascii) > > +struct kernel_param; > +int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp); > +int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp); > + > /* for test only, generally expect drm.debug style macro wrappers */ > #define __pr_debug_cls(cls, fmt, ...) do { \ > BUILD_BUG_ON_MSG(!__builtin_constant_p(cls), \ > @@ -322,6 +332,14 @@ static inline int ddebug_dyndbg_module_param_cb(char *param, char *val, > rowsize, groupsize, buf, len, ascii); \ > } while (0) > > +struct kernel_param; > +static inline int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp) > +{ return 0; } > +static inline int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp) > +{ return 0; } > + > #endif /* !CONFIG_DYNAMIC_DEBUG_CORE */ > > +extern const struct kernel_param_ops param_ops_dyndbg_classes; > + > #endif > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > index 4c27bbe5187e..dd27dc514aa3 100644 > --- a/lib/dynamic_debug.c > +++ b/lib/dynamic_debug.c > @@ -596,6 +596,199 @@ static int ddebug_exec_queries(char *query, const char *modname) > return nfound; > } > > +static int ddebug_apply_class_bitmap(const struct ddebug_classes_bitmap_param *dcp, > + unsigned long inbits) > +{ > +#define QUERY_SIZE 128 > + char query[QUERY_SIZE]; > + const struct ddebug_class_map *map = dcp->map; > + int matches = 0; > + int bi, ct; > + > + v2pr_info("in: 0x%lx on: 0x%lx\n", inbits, *dcp->bits); > + > + for (bi = 0; bi < map->length; bi++) { > + if (test_bit(bi, &inbits) == test_bit(bi, dcp->bits)) > + continue; > + > + snprintf(query, QUERY_SIZE, "class %s %c%s", map->class_names[bi], > + test_bit(bi, &inbits) ? '+' : '-', dcp->flags); > + > + ct = ddebug_exec_queries(query, NULL); > + matches += ct; > + > + v2pr_info("bit_%d: %d matches on class: %s -> 0x%lx\n", bi, > + ct, map->class_names[bi], inbits); > + } > + return matches; > +} > + > +/* support for [+-] symbolic-name boolean list */ > +static int param_set_dyndbg_class_strings(const char *instr, const struct kernel_param *kp) > +{ > + const struct ddebug_classes_bitmap_param *dcp = kp->arg; > + const struct ddebug_class_map *map = dcp->map; > + unsigned long inbits; > + int idx, totct = 0; > + bool wanted; > + char *cls, *p; > + > + cls = kstrdup(instr, GFP_KERNEL); > + p = strchr(cls, '\n'); > + if (p) > + *p = '\0'; > + > + vpr_info("\"%s\" > %s\n", cls, kp->name); > + inbits = *dcp->bits; > + > + for (; cls; cls = p) { > + p = strchr(cls, ','); > + if (p) > + *p++ = '\0'; > + > + if (*cls == '-') { > + wanted = false; > + cls++; > + } else { > + wanted = true; > + if (*cls == '+') > + cls++; > + } > + idx = match_string(map->class_names, map->length, cls); > + if (idx < 0) { > + pr_err("%s unknown to %s\n", cls, kp->name); > + continue; > + } > + > + switch (map->map_type) { > + case DD_CLASS_TYPE_SYMBOLIC: > + if (test_bit(idx, &inbits) == wanted) { > + v3pr_info("no change on %s\n", cls); > + continue; > + } > + inbits ^= BIT(idx); Didn't test this out but the code here confused me. In this case the bit at idx in inbits doesn't match. But you are doing an exclusive OR here. So doesn't that always set it? Shouldn't it be cleared if wanted is false? > + break; > + case DD_CLASS_TYPE_LEVELS: > + /* bitmask must respect classmap ranges, this does not */ > + inbits = (1 << (idx + wanted)); This line also confused me - below in DD_CLASS_TYPE_VERBOSE: case you use the CLASSMAP_BITMASK() which will set all the 'levels' below. So I was expecting that here as well as this is the symbolic level case. I think I'm missing something... > + break; > + default: > + pr_err("illegal map-type value %d\n", map->map_type); > + } > + v2pr_info("%s: bit %d: %s\n", kp->name, idx, map->class_names[idx]); > + totct += ddebug_apply_class_bitmap(dcp, inbits); > + } > + kfree(cls); > + *dcp->bits = inbits; > + vpr_info("total matches: %d\n", totct); > + return 0; > +} > + > +#define CLASSMAP_BITMASK(width) ((1UL << (width)) - 1) > + > +/** > + * param_set_dyndbg_classes - bits => categories >control setter > + * @instr: string echo>d to sysfs > + * @kp: kp->arg has state: bits, map > + * > + * Enable/disable prdbgs by their "category", as given in the > + * arguments to DYNAMIC_DEBUG_CLASSES. > + * > + * Returns: 0 or <0 if error. > + */ > +int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp) > +{ > + const struct ddebug_classes_bitmap_param *dcp = kp->arg; > + const struct ddebug_class_map *map = dcp->map; > + unsigned long inrep; > + int rc, totct = 0; > + > + switch (map->map_type) { > + > + case DD_CLASS_TYPE_SYMBOLIC: > + case DD_CLASS_TYPE_LEVELS: > + /* CSV list of [+-]classnames */ > + return param_set_dyndbg_class_strings(instr, kp); > + > + case DD_CLASS_TYPE_DISJOINT: > + case DD_CLASS_TYPE_VERBOSE: > + /* numeric input */ > + rc = kstrtoul(instr, 0, &inrep); > + if (rc) { > + pr_err("expecting numeric input: %s > %s\n", instr, kp->name); > + return -EINVAL; > + } > + break; > + default: > + pr_err("%s: bad map type: %d\n", kp->name, map->map_type); > + return -EINVAL; > + } > + > + switch (map->map_type) { The second switch here on the same thing as above reads a bit a bit funny to me. Maybe the below can be moved into the first switch block? I guess the 'kstrtoul()' call is common, so I guess you could add an if (DD_CLASS_TYPE_DISJOINT) else {} above. This is a bit of style nitpick I guess. > + case DD_CLASS_TYPE_DISJOINT: > + /* expect bits. mask and warn if too many */ > + if (inrep & ~CLASSMAP_BITMASK(map->length)) { > + pr_warn("%s: input: 0x%lx exceeds mask: 0x%lx, masking\n", > + kp->name, inrep, CLASSMAP_BITMASK(map->length)); > + inrep &= CLASSMAP_BITMASK(map->length); > + } > + break; > + case DD_CLASS_TYPE_VERBOSE: > + /* input is bitpos, of highest verbosity enabled */ > + if (inrep > map->length) { > + pr_warn("%s: verbosity:%ld exceeds range:%d, clamping\n", > + kp->name, inrep, map->length); > + inrep = map->length; > + } > + v2pr_info("VERBOSE: %ld > %s\n", inrep, kp->name); > + inrep = CLASSMAP_BITMASK(inrep + 1); > + break; > + default: > + pr_warn("%s: bad map type: %d\n", kp->name, map->map_type); > + } > + totct += ddebug_apply_class_bitmap(dcp, inrep); > + *dcp->bits = inrep; > + > + vpr_info("%s: total matches: %d\n", kp->name, totct); > + return 0; > +} > +EXPORT_SYMBOL(param_set_dyndbg_classes); > + > +/** > + * param_get_dyndbg_classes - classes reader > + * @buffer: string description of controlled bits -> classes > + * @kp: kp->arg has state: bits, map > + * > + * Reads last written bits, underlying prdbg state may have changed since. > + * Returns: #chars written or <0 on error > + */ > +int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp) > +{ > + const struct ddebug_classes_bitmap_param *dcp = kp->arg; > + const struct ddebug_class_map *map = dcp->map; > + unsigned long val = *dcp->bits; > + > + switch (map->map_type) { > + case DD_CLASS_TYPE_SYMBOLIC: > + case DD_CLASS_TYPE_DISJOINT: > + case DD_CLASS_TYPE_LEVELS: > + return scnprintf(buffer, PAGE_SIZE, "0x%lx\n", val); > + case DD_CLASS_TYPE_VERBOSE: > + /* convert internal bits to a level */ > + return scnprintf(buffer, PAGE_SIZE, "%lu\n", > + find_first_zero_bit(&val, map->length) - 1); > + default: > + return -1; > + } > +} > +EXPORT_SYMBOL(param_get_dyndbg_classes); > + > +const struct kernel_param_ops param_ops_dyndbg_classes = { > + .set = param_set_dyndbg_classes, > + .get = param_get_dyndbg_classes, > +}; > +EXPORT_SYMBOL(param_ops_dyndbg_classes); > + > #define PREFIX_SIZE 64 > > static int remaining(int wrote)
On Fri, Jul 22, 2022 at 2:33 PM Jason Baron <jbaron@akamai.com> wrote: > > > > On 7/20/22 11:32, Jim Cromie wrote: > > Add kernel_param_ops and callbacks to apply a class-map to a > > sysfs-node, which then can control classes defined in that class-map. > > This supports uses like: > > > > echo 0x3 > /sys/module/drm/parameters/debug > > > > IE add these: > > > > - int param_set_dyndbg_classes() > > - int param_get_dyndbg_classes() > > - struct kernel_param_ops param_ops_dyndbg_classes > > > > Following the model of kernel/params.c STANDARD_PARAM_DEFS, these are > > non-static and exported. This might be unnecessary here. > > > > get/set use an augmented kernel_param; the arg refs a new struct > > ddebug_classes_bitmap_param, initialized by DYNAMIC_DEBUG_CLASSBITS > > macro, which contains: > > > > BITS: a pointer to the user module's ulong holding the bits/state. By > > ref'g the client's bit-state _var, we coordinate with existing code > > (such as drm_debug_enabled) which uses the _var, so it works > > unchanged, even as the foundation is switched out underneath it.. > > Using a ulong allows use of BIT() etc. > > > > FLAGS: dyndbg.flags toggled by changes to bitmap. Usually just "p". > > > > MAP: a pointer to struct ddebug_classes_map, which maps those > > class-names to .class_ids 0..N that the module is using. This > > class-map is declared & initialized by DEFINE_DYNDBG_CLASSMAP. > > > > map-type: add support here for DD_CLASS_DISJOINT, DD_CLASS_VERBOSE. > > > > These 2 class-types both expect an integer; _DISJOINT treats input > > like a bit-vector (ala drm.debug), and sets each bit accordingly. > > > > _VERBOSE treats input like a bit-pos:N, then sets bits(0..N)=1, and > > bits(N+1..max)=0. This applies (bit<N) semantics on top of disjoint > > bits. > > > > cases DD_CLASS_SYMBOLIC, DD_CLASS_LEVELS are included for the complete > > picture, with commented out call to a following commit. > > > > NOTES: > > > > this now includes SYMBOLIC/LEVELS support, too tedious to keep > > separate thru all the tweaking. > > > > get-param undoes the bit-pos -> bitmap transform that set-param does > > on VERBOSE inputs, this gives the read-what-was-written property. > > > > _VERBOSE is overlay on _DISJOINT: > > > > verbose-maps still need class-names, even though theyre not usable at > > the sysfs interface (unlike with _SYMBOLIC/_LEVELS). > > > > - It must have a "V0" name, > > something below "V1" to turn "V1" off. > > __pr_debug_cls(V0,..) is printk, don't do that. > > > > - "class names" is required at the >control interface. > > - relative levels are not enforced at >control > > > > IOW this is possible, and maybe confusing: > > > > echo class V3 +p > control > > echo class V1 -p > control > > > > IMO thats ok, relative verbosity is an interface property. > > > > Signed-off-by: Jim Cromie <jim.cromie@gmail.com> > > --- > > . drop kp->mod->name as unneeded (build-dependent) <lkp> > > --- > > include/linux/dynamic_debug.h | 18 ++++ > > lib/dynamic_debug.c | 193 ++++++++++++++++++++++++++++++++++ > > 2 files changed, 211 insertions(+) > > > > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h > > index f57076e02767..b50bdd5c8184 100644 > > --- a/include/linux/dynamic_debug.h > > +++ b/include/linux/dynamic_debug.h > > @@ -113,6 +113,12 @@ struct ddebug_class_map { > > #define NUM_TYPE_ARGS(eltype, ...) \ > > (sizeof((eltype[]) {__VA_ARGS__}) / sizeof(eltype)) > > > > +struct ddebug_classes_bitmap_param { > > + unsigned long *bits; > > + char flags[8]; > > + const struct ddebug_class_map *map; > > +}; > > + > > #if defined(CONFIG_DYNAMIC_DEBUG_CORE) > > > > int ddebug_add_module(struct _ddebug *tab, unsigned int num_debugs, > > @@ -274,6 +280,10 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > > KERN_DEBUG, prefix_str, prefix_type, \ > > rowsize, groupsize, buf, len, ascii) > > > > +struct kernel_param; > > +int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp); > > +int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp); > > + > > /* for test only, generally expect drm.debug style macro wrappers */ > > #define __pr_debug_cls(cls, fmt, ...) do { \ > > BUILD_BUG_ON_MSG(!__builtin_constant_p(cls), \ > > @@ -322,6 +332,14 @@ static inline int ddebug_dyndbg_module_param_cb(char *param, char *val, > > rowsize, groupsize, buf, len, ascii); \ > > } while (0) > > > > +struct kernel_param; > > +static inline int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp) > > +{ return 0; } > > +static inline int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp) > > +{ return 0; } > > + > > #endif /* !CONFIG_DYNAMIC_DEBUG_CORE */ > > > > +extern const struct kernel_param_ops param_ops_dyndbg_classes; > > + > > #endif > > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > > index 4c27bbe5187e..dd27dc514aa3 100644 > > --- a/lib/dynamic_debug.c > > +++ b/lib/dynamic_debug.c > > @@ -596,6 +596,199 @@ static int ddebug_exec_queries(char *query, const char *modname) > > return nfound; > > } > > > > +static int ddebug_apply_class_bitmap(const struct ddebug_classes_bitmap_param *dcp, > > + unsigned long inbits) > > +{ > > +#define QUERY_SIZE 128 > > + char query[QUERY_SIZE]; > > + const struct ddebug_class_map *map = dcp->map; > > + int matches = 0; > > + int bi, ct; > > + > > + v2pr_info("in: 0x%lx on: 0x%lx\n", inbits, *dcp->bits); > > + > > + for (bi = 0; bi < map->length; bi++) { > > + if (test_bit(bi, &inbits) == test_bit(bi, dcp->bits)) > > + continue; > > + > > + snprintf(query, QUERY_SIZE, "class %s %c%s", map->class_names[bi], > > + test_bit(bi, &inbits) ? '+' : '-', dcp->flags); > > + > > + ct = ddebug_exec_queries(query, NULL); > > + matches += ct; > > + > > + v2pr_info("bit_%d: %d matches on class: %s -> 0x%lx\n", bi, > > + ct, map->class_names[bi], inbits); > > + } > > + return matches; > > +} > > + > > +/* support for [+-] symbolic-name boolean list */ > > +static int param_set_dyndbg_class_strings(const char *instr, const struct kernel_param *kp) > > +{ > > + const struct ddebug_classes_bitmap_param *dcp = kp->arg; > > + const struct ddebug_class_map *map = dcp->map; > > + unsigned long inbits; > > + int idx, totct = 0; > > + bool wanted; > > + char *cls, *p; > > + > > + cls = kstrdup(instr, GFP_KERNEL); > > + p = strchr(cls, '\n'); > > + if (p) > > + *p = '\0'; > > + > > + vpr_info("\"%s\" > %s\n", cls, kp->name); > > + inbits = *dcp->bits; > > + > > + for (; cls; cls = p) { > > + p = strchr(cls, ','); > > + if (p) > > + *p++ = '\0'; > > + > > + if (*cls == '-') { > > + wanted = false; > > + cls++; > > + } else { > > + wanted = true; > > + if (*cls == '+') > > + cls++; > > + } > > + idx = match_string(map->class_names, map->length, cls); > > + if (idx < 0) { > > + pr_err("%s unknown to %s\n", cls, kp->name); > > + continue; > > + } > > + > > + switch (map->map_type) { > > + case DD_CLASS_TYPE_SYMBOLIC: > > + if (test_bit(idx, &inbits) == wanted) { > > + v3pr_info("no change on %s\n", cls); > > + continue; > > + } > > + inbits ^= BIT(idx); > > Didn't test this out but the code here confused me. In this case the bit at idx > in inbits doesn't match. But you are doing an exclusive OR here. So doesn't that > always set it? Shouldn't it be cleared if wanted is false? > I think the trouble is - in part - inbits varname; it starts with the old bit vector value - inbits = *dcp->bits; // i'll add comment here 2nd, this is while parsing the csv of classnames, and evaluating 1 bit/class at a time here, and only valid class-names and wanted therefore only pertains to classes/bits that need toggled, wanted = 1/0 based on +/- setting for that class. > > > + break; > > + case DD_CLASS_TYPE_LEVELS: > > + /* bitmask must respect classmap ranges, this does not */ > > + inbits = (1 << (idx + wanted)); > > This line also confused me - below in DD_CLASS_TYPE_VERBOSE: case you use the > CLASSMAP_BITMASK() which will set all the 'levels' below. So I was expecting > that here as well as this is the symbolic level case. I think I'm missing > something... So this is a bit inscrutable - and the comment needs refresh. again, we are taking classnames here, so are working a single class-id, this time the bits are not independent, as they were above. idx is the valid class-id pertaining to the classname - I'll pick a better varname here too so inbits = (1<< ( cls_id + wanted ? 1 : 0 )) forces all bits below cls_id on, and maybe cls_id too. at bottom of the classnames loop, apply-bitmap applies the bits. it applies only changes made to inbits, by re-fetching orig dcp->bits and comparing. Note also the usage diffs for the 2 _NAMED styles echo DRM_UT_CORE,-DRM_UT_KMS,+DRM_UT_DRIVER > debug_catnames echo +NV_TRACE > nv_debug_level_names while this would be weird / makework / stress-test echo +V7,-V1,+V7,-V1 > /sys/module/test_dynamic_debug/p_level_names Ive been reworking the set - including your enum suggestions, also adding 0010-dyndbg-cleanup-local-vars-in-ddebug_init.patch 0011-dyndbg-create-and-use-struct-_ddebug_info.patch - this gathers __dyndbg and __dyndbg_classes sections - maybe infos, or something more subsystem-state-ey any suggestions ? I'll change varnames I mentioned, and add comments. I'll add comments derived from this explanation (hope it clears things up) and have a few things to sort and retest, I'll send a new rev shortly.
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h index f57076e02767..b50bdd5c8184 100644 --- a/include/linux/dynamic_debug.h +++ b/include/linux/dynamic_debug.h @@ -113,6 +113,12 @@ struct ddebug_class_map { #define NUM_TYPE_ARGS(eltype, ...) \ (sizeof((eltype[]) {__VA_ARGS__}) / sizeof(eltype)) +struct ddebug_classes_bitmap_param { + unsigned long *bits; + char flags[8]; + const struct ddebug_class_map *map; +}; + #if defined(CONFIG_DYNAMIC_DEBUG_CORE) int ddebug_add_module(struct _ddebug *tab, unsigned int num_debugs, @@ -274,6 +280,10 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, KERN_DEBUG, prefix_str, prefix_type, \ rowsize, groupsize, buf, len, ascii) +struct kernel_param; +int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp); +int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp); + /* for test only, generally expect drm.debug style macro wrappers */ #define __pr_debug_cls(cls, fmt, ...) do { \ BUILD_BUG_ON_MSG(!__builtin_constant_p(cls), \ @@ -322,6 +332,14 @@ static inline int ddebug_dyndbg_module_param_cb(char *param, char *val, rowsize, groupsize, buf, len, ascii); \ } while (0) +struct kernel_param; +static inline int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp) +{ return 0; } +static inline int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp) +{ return 0; } + #endif /* !CONFIG_DYNAMIC_DEBUG_CORE */ +extern const struct kernel_param_ops param_ops_dyndbg_classes; + #endif diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 4c27bbe5187e..dd27dc514aa3 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -596,6 +596,199 @@ static int ddebug_exec_queries(char *query, const char *modname) return nfound; } +static int ddebug_apply_class_bitmap(const struct ddebug_classes_bitmap_param *dcp, + unsigned long inbits) +{ +#define QUERY_SIZE 128 + char query[QUERY_SIZE]; + const struct ddebug_class_map *map = dcp->map; + int matches = 0; + int bi, ct; + + v2pr_info("in: 0x%lx on: 0x%lx\n", inbits, *dcp->bits); + + for (bi = 0; bi < map->length; bi++) { + if (test_bit(bi, &inbits) == test_bit(bi, dcp->bits)) + continue; + + snprintf(query, QUERY_SIZE, "class %s %c%s", map->class_names[bi], + test_bit(bi, &inbits) ? '+' : '-', dcp->flags); + + ct = ddebug_exec_queries(query, NULL); + matches += ct; + + v2pr_info("bit_%d: %d matches on class: %s -> 0x%lx\n", bi, + ct, map->class_names[bi], inbits); + } + return matches; +} + +/* support for [+-] symbolic-name boolean list */ +static int param_set_dyndbg_class_strings(const char *instr, const struct kernel_param *kp) +{ + const struct ddebug_classes_bitmap_param *dcp = kp->arg; + const struct ddebug_class_map *map = dcp->map; + unsigned long inbits; + int idx, totct = 0; + bool wanted; + char *cls, *p; + + cls = kstrdup(instr, GFP_KERNEL); + p = strchr(cls, '\n'); + if (p) + *p = '\0'; + + vpr_info("\"%s\" > %s\n", cls, kp->name); + inbits = *dcp->bits; + + for (; cls; cls = p) { + p = strchr(cls, ','); + if (p) + *p++ = '\0'; + + if (*cls == '-') { + wanted = false; + cls++; + } else { + wanted = true; + if (*cls == '+') + cls++; + } + idx = match_string(map->class_names, map->length, cls); + if (idx < 0) { + pr_err("%s unknown to %s\n", cls, kp->name); + continue; + } + + switch (map->map_type) { + case DD_CLASS_TYPE_SYMBOLIC: + if (test_bit(idx, &inbits) == wanted) { + v3pr_info("no change on %s\n", cls); + continue; + } + inbits ^= BIT(idx); + break; + case DD_CLASS_TYPE_LEVELS: + /* bitmask must respect classmap ranges, this does not */ + inbits = (1 << (idx + wanted)); + break; + default: + pr_err("illegal map-type value %d\n", map->map_type); + } + v2pr_info("%s: bit %d: %s\n", kp->name, idx, map->class_names[idx]); + totct += ddebug_apply_class_bitmap(dcp, inbits); + } + kfree(cls); + *dcp->bits = inbits; + vpr_info("total matches: %d\n", totct); + return 0; +} + +#define CLASSMAP_BITMASK(width) ((1UL << (width)) - 1) + +/** + * param_set_dyndbg_classes - bits => categories >control setter + * @instr: string echo>d to sysfs + * @kp: kp->arg has state: bits, map + * + * Enable/disable prdbgs by their "category", as given in the + * arguments to DYNAMIC_DEBUG_CLASSES. + * + * Returns: 0 or <0 if error. + */ +int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp) +{ + const struct ddebug_classes_bitmap_param *dcp = kp->arg; + const struct ddebug_class_map *map = dcp->map; + unsigned long inrep; + int rc, totct = 0; + + switch (map->map_type) { + + case DD_CLASS_TYPE_SYMBOLIC: + case DD_CLASS_TYPE_LEVELS: + /* CSV list of [+-]classnames */ + return param_set_dyndbg_class_strings(instr, kp); + + case DD_CLASS_TYPE_DISJOINT: + case DD_CLASS_TYPE_VERBOSE: + /* numeric input */ + rc = kstrtoul(instr, 0, &inrep); + if (rc) { + pr_err("expecting numeric input: %s > %s\n", instr, kp->name); + return -EINVAL; + } + break; + default: + pr_err("%s: bad map type: %d\n", kp->name, map->map_type); + return -EINVAL; + } + + switch (map->map_type) { + case DD_CLASS_TYPE_DISJOINT: + /* expect bits. mask and warn if too many */ + if (inrep & ~CLASSMAP_BITMASK(map->length)) { + pr_warn("%s: input: 0x%lx exceeds mask: 0x%lx, masking\n", + kp->name, inrep, CLASSMAP_BITMASK(map->length)); + inrep &= CLASSMAP_BITMASK(map->length); + } + break; + case DD_CLASS_TYPE_VERBOSE: + /* input is bitpos, of highest verbosity enabled */ + if (inrep > map->length) { + pr_warn("%s: verbosity:%ld exceeds range:%d, clamping\n", + kp->name, inrep, map->length); + inrep = map->length; + } + v2pr_info("VERBOSE: %ld > %s\n", inrep, kp->name); + inrep = CLASSMAP_BITMASK(inrep + 1); + break; + default: + pr_warn("%s: bad map type: %d\n", kp->name, map->map_type); + } + totct += ddebug_apply_class_bitmap(dcp, inrep); + *dcp->bits = inrep; + + vpr_info("%s: total matches: %d\n", kp->name, totct); + return 0; +} +EXPORT_SYMBOL(param_set_dyndbg_classes); + +/** + * param_get_dyndbg_classes - classes reader + * @buffer: string description of controlled bits -> classes + * @kp: kp->arg has state: bits, map + * + * Reads last written bits, underlying prdbg state may have changed since. + * Returns: #chars written or <0 on error + */ +int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp) +{ + const struct ddebug_classes_bitmap_param *dcp = kp->arg; + const struct ddebug_class_map *map = dcp->map; + unsigned long val = *dcp->bits; + + switch (map->map_type) { + case DD_CLASS_TYPE_SYMBOLIC: + case DD_CLASS_TYPE_DISJOINT: + case DD_CLASS_TYPE_LEVELS: + return scnprintf(buffer, PAGE_SIZE, "0x%lx\n", val); + case DD_CLASS_TYPE_VERBOSE: + /* convert internal bits to a level */ + return scnprintf(buffer, PAGE_SIZE, "%lu\n", + find_first_zero_bit(&val, map->length) - 1); + default: + return -1; + } +} +EXPORT_SYMBOL(param_get_dyndbg_classes); + +const struct kernel_param_ops param_ops_dyndbg_classes = { + .set = param_set_dyndbg_classes, + .get = param_get_dyndbg_classes, +}; +EXPORT_SYMBOL(param_ops_dyndbg_classes); + #define PREFIX_SIZE 64 static int remaining(int wrote)
Add kernel_param_ops and callbacks to apply a class-map to a sysfs-node, which then can control classes defined in that class-map. This supports uses like: echo 0x3 > /sys/module/drm/parameters/debug IE add these: - int param_set_dyndbg_classes() - int param_get_dyndbg_classes() - struct kernel_param_ops param_ops_dyndbg_classes Following the model of kernel/params.c STANDARD_PARAM_DEFS, these are non-static and exported. This might be unnecessary here. get/set use an augmented kernel_param; the arg refs a new struct ddebug_classes_bitmap_param, initialized by DYNAMIC_DEBUG_CLASSBITS macro, which contains: BITS: a pointer to the user module's ulong holding the bits/state. By ref'g the client's bit-state _var, we coordinate with existing code (such as drm_debug_enabled) which uses the _var, so it works unchanged, even as the foundation is switched out underneath it.. Using a ulong allows use of BIT() etc. FLAGS: dyndbg.flags toggled by changes to bitmap. Usually just "p". MAP: a pointer to struct ddebug_classes_map, which maps those class-names to .class_ids 0..N that the module is using. This class-map is declared & initialized by DEFINE_DYNDBG_CLASSMAP. map-type: add support here for DD_CLASS_DISJOINT, DD_CLASS_VERBOSE. These 2 class-types both expect an integer; _DISJOINT treats input like a bit-vector (ala drm.debug), and sets each bit accordingly. _VERBOSE treats input like a bit-pos:N, then sets bits(0..N)=1, and bits(N+1..max)=0. This applies (bit<N) semantics on top of disjoint bits. cases DD_CLASS_SYMBOLIC, DD_CLASS_LEVELS are included for the complete picture, with commented out call to a following commit. NOTES: this now includes SYMBOLIC/LEVELS support, too tedious to keep separate thru all the tweaking. get-param undoes the bit-pos -> bitmap transform that set-param does on VERBOSE inputs, this gives the read-what-was-written property. _VERBOSE is overlay on _DISJOINT: verbose-maps still need class-names, even though theyre not usable at the sysfs interface (unlike with _SYMBOLIC/_LEVELS). - It must have a "V0" name, something below "V1" to turn "V1" off. __pr_debug_cls(V0,..) is printk, don't do that. - "class names" is required at the >control interface. - relative levels are not enforced at >control IOW this is possible, and maybe confusing: echo class V3 +p > control echo class V1 -p > control IMO thats ok, relative verbosity is an interface property. Signed-off-by: Jim Cromie <jim.cromie@gmail.com> --- . drop kp->mod->name as unneeded (build-dependent) <lkp> --- include/linux/dynamic_debug.h | 18 ++++ lib/dynamic_debug.c | 193 ++++++++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+)