Re: [PATCH v2 16/59] dyndbg-API: remove DD_CLASS_TYPE_(DISJOINT|LEVEL)_NAMES and code

[Louis Chauvet <louis.chauvet@bootlin.com>]

Le 20/03/2025 à 19:51, Jim Cromie a écrit :
> Remove the DD_CLASS_TYPE_*_NAMES classmap types and code.
> 
> These 2 classmap types accept class names at the PARAM interface, for
> example:
> 
>    echo +DRM_UT_CORE,-DRM_UT_KMS > /sys/module/drm/parameters/debug_names
> 
> The code works, but its only used by test-dynamic-debug, and wasn't
> asked for by anyone else, so reduce test-surface, and simplify things.
>
> also rename enum class_map_type to enum ddebug_class_map_type.

Nitpick: s/also/Also/

There were no documentation about those features?

> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> ---
>   include/linux/dynamic_debug.h |  23 ++------
>   lib/dynamic_debug.c           | 102 +++-------------------------------
>   lib/test_dynamic_debug.c      |  26 ---------
>   3 files changed, 14 insertions(+), 137 deletions(-)
> 
> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> index 82eabaa6e827..9fb38d79216e 100644
> --- a/include/linux/dynamic_debug.h
> +++ b/include/linux/dynamic_debug.h
> @@ -60,27 +60,16 @@ struct _ddebug {
>   #endif
>   } __attribute__((aligned(8)));
>   
> -enum class_map_type {
> +enum ddebug_class_map_type {
>   	DD_CLASS_TYPE_DISJOINT_BITS,
>   	/**
> -	 * DD_CLASS_TYPE_DISJOINT_BITS: classes are independent, one per bit.
> -	 * expecting hex input. Built for drm.debug, basis for other types.
> +	 * DD_CLASS_TYPE_DISJOINT_BITS: classes are independent, mapped to bits[0..N].
> +	 * Expects hex input. Built for drm.debug, basis for other types.
>   	 */
>   	DD_CLASS_TYPE_LEVEL_NUM,
>   	/**
> -	 * DD_CLASS_TYPE_LEVEL_NUM: input is numeric level, 0-N.
> -	 * N turns on just bits N-1 .. 0, so N=0 turns all bits off.
> -	 */
> -	DD_CLASS_TYPE_DISJOINT_NAMES,
> -	/**
> -	 * DD_CLASS_TYPE_DISJOINT_NAMES: input is a CSV of [+-]CLASS_NAMES,
> -	 * classes are independent, like _DISJOINT_BITS.
> -	 */
> -	DD_CLASS_TYPE_LEVEL_NAMES,
> -	/**
> -	 * DD_CLASS_TYPE_LEVEL_NAMES: input is a CSV of [+-]CLASS_NAMES,
> -	 * intended for names like: INFO,DEBUG,TRACE, with a module prefix
> -	 * avoid EMERG,ALERT,CRIT,ERR,WARNING: they're not debug
> +	 * DD_CLASS_TYPE_LEVEL_NUM: input is numeric level, 0..N.
> +	 * Input N turns on bits 0..N-1
>   	 */
>   };
>   
> @@ -90,7 +79,7 @@ struct ddebug_class_map {
>   	const char **class_names;
>   	const int length;
>   	const int base;		/* index of 1st .class_id, allows split/shared space */
> -	enum class_map_type map_type;
> +	enum ddebug_class_map_type map_type;
>   };
>   
>   /**
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 192783ff7b98..5df9cc732290 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -655,76 +655,6 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
>   
>   #define CLASSMAP_BITMASK(width) ((1UL << (width)) - 1)
>   
> -/* accept comma-separated-list of [+-] classnames */
> -static int param_set_dyndbg_classnames(const char *instr, const struct kernel_param *kp)
> -{
> -	const struct ddebug_class_param *dcp = kp->arg;
> -	const struct ddebug_class_map *map = dcp->map;
> -	unsigned long curr_bits, old_bits;
> -	char *cl_str, *p, *tmp;
> -	int cls_id, totct = 0;
> -	bool wanted;
> -
> -	cl_str = tmp = kstrdup_and_replace(instr, '\n', '\0', GFP_KERNEL);
> -	if (!tmp)
> -		return -ENOMEM;
> -
> -	/* start with previously set state-bits, then modify */
> -	curr_bits = old_bits = *dcp->bits;
> -	vpr_info("\"%s\" > %s:0x%lx\n", cl_str, KP_NAME(kp), curr_bits);
> -
> -	for (; cl_str; cl_str = p) {
> -		p = strchr(cl_str, ',');
> -		if (p)
> -			*p++ = '\0';
> -
> -		if (*cl_str == '-') {
> -			wanted = false;
> -			cl_str++;
> -		} else {
> -			wanted = true;
> -			if (*cl_str == '+')
> -				cl_str++;
> -		}
> -		cls_id = match_string(map->class_names, map->length, cl_str);
> -		if (cls_id < 0) {
> -			pr_err("%s unknown to %s\n", cl_str, KP_NAME(kp));
> -			continue;
> -		}
> -
> -		/* have one or more valid class_ids of one *_NAMES type */
> -		switch (map->map_type) {
> -		case DD_CLASS_TYPE_DISJOINT_NAMES:
> -			/* the +/- pertains to a single bit */
> -			if (test_bit(cls_id, &curr_bits) == wanted) {
> -				v3pr_info("no change on %s\n", cl_str);
> -				continue;
> -			}
> -			curr_bits ^= BIT(cls_id);
> -			totct += ddebug_apply_class_bitmap(dcp, &curr_bits, *dcp->bits, NULL);
> -			*dcp->bits = curr_bits;
> -			v2pr_info("%s: changed bit %d:%s\n", KP_NAME(kp), cls_id,
> -				  map->class_names[cls_id]);
> -			break;
> -		case DD_CLASS_TYPE_LEVEL_NAMES:
> -			/* cls_id = N in 0..max. wanted +/- determines N or N-1 */
> -			old_bits = CLASSMAP_BITMASK(*dcp->lvl);
> -			curr_bits = CLASSMAP_BITMASK(cls_id + (wanted ? 1 : 0 ));
> -
> -			totct += ddebug_apply_class_bitmap(dcp, &curr_bits, old_bits, NULL);
> -			*dcp->lvl = (cls_id + (wanted ? 1 : 0));
> -			v2pr_info("%s: changed bit-%d: \"%s\" %lx->%lx\n", KP_NAME(kp), cls_id,
> -				  map->class_names[cls_id], old_bits, curr_bits);
> -			break;
> -		default:
> -			pr_err("illegal map-type value %d\n", map->map_type);
> -		}
> -	}
> -	kfree(tmp);
> -	vpr_info("total matches: %d\n", totct);
> -	return 0;
> -}
> -
>   static int param_set_dyndbg_module_classes(const char *instr,
>   					   const struct kernel_param *kp,
>   					   const char *modnm)
> @@ -733,29 +663,17 @@ static int param_set_dyndbg_module_classes(const char *instr,
>   	const struct ddebug_class_map *map = dcp->map;
>   	unsigned long inrep, new_bits, old_bits;
>   	int rc, totct = 0;
> -
> -	switch (map->map_type) {
> -
> -	case DD_CLASS_TYPE_DISJOINT_NAMES:
> -	case DD_CLASS_TYPE_LEVEL_NAMES:
> -		/* handle [+-]classnames list separately, we are done here */
> -		return param_set_dyndbg_classnames(instr, kp);
> -
> -	case DD_CLASS_TYPE_DISJOINT_BITS:
> -	case DD_CLASS_TYPE_LEVEL_NUM:
> -		/* numeric input, accept and fall-thru */
> -		rc = kstrtoul(instr, 0, &inrep);
> -		if (rc) {
> -			pr_err("expecting numeric input: %s > %s\n", instr, KP_NAME(kp));
> -			return -EINVAL;
> -		}
> -		break;
> -	default:
> -		pr_err("%s: bad map type: %d\n", KP_NAME(kp), map->map_type);
> +	char *nl;
> +
> +	rc = kstrtoul(instr, 0, &inrep);
> +	if (rc) {
> +		nl = strchr(instr, '\n');
> +		if (nl)
> +			*nl = '\0';
> +		pr_err("expecting numeric input, not: %s > %s\n", instr, KP_NAME(kp));
>   		return -EINVAL;
>   	}
>   
> -	/* only _BITS,_NUM (numeric) map-types get here */
>   	switch (map->map_type) {
>   	case DD_CLASS_TYPE_DISJOINT_BITS:
>   		/* expect bits. mask and warn if too many */
> @@ -821,12 +739,8 @@ int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp)
>   	const struct ddebug_class_map *map = dcp->map;
>   
>   	switch (map->map_type) {
> -
> -	case DD_CLASS_TYPE_DISJOINT_NAMES:
>   	case DD_CLASS_TYPE_DISJOINT_BITS:
>   		return scnprintf(buffer, PAGE_SIZE, "0x%lx\n", *dcp->bits);
> -
> -	case DD_CLASS_TYPE_LEVEL_NAMES:
>   	case DD_CLASS_TYPE_LEVEL_NUM:
>   		return scnprintf(buffer, PAGE_SIZE, "%ld\n", *dcp->lvl);
>   	default:
> diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
> index 396144cf351b..74d183ebf3e0 100644
> --- a/lib/test_dynamic_debug.c
> +++ b/lib/test_dynamic_debug.c
> @@ -74,13 +74,6 @@ DECLARE_DYNDBG_CLASSMAP(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS, 0,
>   DD_SYS_WRAP(disjoint_bits, p);
>   DD_SYS_WRAP(disjoint_bits, T);
>   
> -/* symbolic input, independent bits */
> -enum cat_disjoint_names { LOW = 10, MID, HI };
> -DECLARE_DYNDBG_CLASSMAP(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES, 10,
> -			"LOW", "MID", "HI");
> -DD_SYS_WRAP(disjoint_names, p);
> -DD_SYS_WRAP(disjoint_names, T);
> -
>   /* numeric verbosity, V2 > V1 related */
>   enum cat_level_num { V0 = 14, V1, V2, V3, V4, V5, V6, V7 };
>   DECLARE_DYNDBG_CLASSMAP(map_level_num, DD_CLASS_TYPE_LEVEL_NUM, 14,
> @@ -88,13 +81,6 @@ DECLARE_DYNDBG_CLASSMAP(map_level_num, DD_CLASS_TYPE_LEVEL_NUM, 14,
>   DD_SYS_WRAP(level_num, p);
>   DD_SYS_WRAP(level_num, T);
>   
> -/* symbolic verbosity */
> -enum cat_level_names { L0 = 22, L1, L2, L3, L4, L5, L6, L7 };
> -DECLARE_DYNDBG_CLASSMAP(map_level_names, DD_CLASS_TYPE_LEVEL_NAMES, 22,
> -			"L0", "L1", "L2", "L3", "L4", "L5", "L6", "L7");
> -DD_SYS_WRAP(level_names, p);
> -DD_SYS_WRAP(level_names, T);
> -
>   /* stand-in for all pr_debug etc */
>   #define prdbg(SYM) __pr_debug_cls(SYM, #SYM " msg\n")
>   
> @@ -102,10 +88,6 @@ static void do_cats(void)
>   {
>   	pr_debug("doing categories\n");
>   
> -	prdbg(LOW);
> -	prdbg(MID);
> -	prdbg(HI);
> -
>   	prdbg(D2_CORE);
>   	prdbg(D2_DRIVER);
>   	prdbg(D2_KMS);
> @@ -129,14 +111,6 @@ static void do_levels(void)
>   	prdbg(V5);
>   	prdbg(V6);
>   	prdbg(V7);
> -
> -	prdbg(L1);
> -	prdbg(L2);
> -	prdbg(L3);
> -	prdbg(L4);
> -	prdbg(L5);
> -	prdbg(L6);
> -	prdbg(L7);
>   }
>   
>   static void do_prints(void)

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com