linux-kernel - Re: [PATCH v9 10/10] drm: use DEFINE_DYNAMIC_DEBUG_TRACE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3a55ab9c-8109-8025-21e3-e3635bd891b2@akamai.com>
Date:   Wed, 3 Nov 2021 11:58:10 -0400
From:   Jason Baron <jbaron@...mai.com>
To:     Jim Cromie <jim.cromie@...il.com>, gregkh@...uxfoundation.org,
        linux-kernel@...r.kernel.org, linux@...musvillemoes.dk,
        daniel.vetter@...ll.ch, seanpaul@...omium.org,
        dri-devel@...ts.freedesktop.org, amd-gfx@...ts.freedesktop.org,
        intel-gvt-dev@...ts.freedesktop.org,
        intel-gfx@...ts.freedesktop.org
Subject: Re: [PATCH v9 10/10] drm: use DEFINE_DYNAMIC_DEBUG_TRACE_CATEGORIES
 bitmap to tracefs



On 10/27/21 12:36 AM, Jim Cromie wrote:
> Use new macro to create a sysfs control bitmap knob to control
> print-to-trace in: /sys/module/drm/parameters/trace
> 
> todo: reconsider this api, ie a single macro expecting both debug &
> trace terms (2 each), followed by a single description and the
> bitmap-spec::
> 
> Good: declares bitmap once for both interfaces
> 
> Bad: arg-type/count handling (expecting 4 args) is ugly,
>      especially preceding the bitmap-init var-args.
> 

Hi Jim,

I agree having the bitmap declared twice seems redundant. But I like having fewer args and not necessarily combining the trace/log variants of
DEBUG_CATEGORIES. hmmm...what if the DEFINE_DYNAMIC_DEBUG_CATEGORIES() took a pointer to the array of struct dyndbg_bitdesc map[] directly as the
final argument instead of the __VA_ARGS__? Then, we could just declare the map once?

Thanks,

-Jason

> Signed-off-by: Jim Cromie <jim.cromie@...il.com>
> ---
>  drivers/gpu/drm/drm_print.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index ce662d0f339b..7b49fbc5e21d 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -73,6 +73,25 @@ DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug, __drm_debug,
>  				[7] = { DRM_DBG_CAT_LEASE },
>  				[8] = { DRM_DBG_CAT_DP },
>  				[9] = { DRM_DBG_CAT_DRMRES });
> +
> +#ifdef CONFIG_TRACING
> +unsigned long __drm_trace;
> +EXPORT_SYMBOL(__drm_trace);
> +DEFINE_DYNAMIC_DEBUG_TRACE_CATEGORIES(trace, __drm_trace,
> +				      DRM_DEBUG_DESC,
> +				      [0] = { DRM_DBG_CAT_CORE },
> +				      [1] = { DRM_DBG_CAT_DRIVER },
> +				      [2] = { DRM_DBG_CAT_KMS },
> +				      [3] = { DRM_DBG_CAT_PRIME },
> +				      [4] = { DRM_DBG_CAT_ATOMIC },
> +				      [5] = { DRM_DBG_CAT_VBL },
> +				      [6] = { DRM_DBG_CAT_STATE },
> +				      [7] = { DRM_DBG_CAT_LEASE },
> +				      [8] = { DRM_DBG_CAT_DP },
> +				      [9] = { DRM_DBG_CAT_DRMRES });
> +
> +struct trace_array *trace_arr;
> +#endif
>  #endif
>  
>  void __drm_puts_coredump(struct drm_printer *p, const char *str)
>