linux-kernel - Re: [Freedreno] [PATCH] drm/msm/dp: Move debugfs files into subdirectory

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: <f72263e0d4c118653fff8b1341dc487b@codeaurora.org>
Date:   Fri, 15 Oct 2021 16:53:48 -0700
From:   abhinavk@...eaurora.org
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
Cc:     Rob Clark <robdclark@...il.com>,
        Stephen Boyd <swboyd@...omium.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Sean Paul <sean@...rly.run>, David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>, linux-arm-msm@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: Re: [Freedreno] [PATCH] drm/msm/dp: Move debugfs files into
 subdirectory

On 2021-10-15 16:17, Bjorn Andersson wrote:
> In the cleanup path of the MSM DP driver the DP driver's debugfs files
> are destroyed by invoking debugfs_remove_recursive() on debug->root,
> which during initialization has been set to minor->debugfs_root.
> 
> To allow cleaning up the DP driver's debugfs files either each dentry
> needs to be kept track of or the files needs to be put in a 
> subdirectory
> which can be removed in one go.
> 
> By choosing to put the debugfs files in a subdirectory, based on the
> name of the associated connector this also solves the problem that 
> these
> names would collide as support for multiple DP instances are 
> introduced.
> 
> One alternative solution to the problem with colliding file names would
> have been to put keep track of the individual files and put them under
> the connector's debugfs directory. But while the drm_connector has been
> allocated, its associated debugfs directory has not been created at the
> time of initialization of the dp_debug.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@...aro.org>

I have been thinking about this problem ever since multi-DP has been 
posted :)
Creating sub-directories seems right but at the moment it looks like IGT 
which
uses these debugfs nodes doesnt check sub-directories:

https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tools/msm_dp_compliance.c#L215

It looks for the DP debugfs nodes under /sys/kernel/debug/dri/*/

We have to fix IGT too to be able to handle multi-DP cases. I will try 
to come up
with a proposal to address this.

Till then, can we go with the other solution to keep track of the 
dentries?

> ---
> 
> This depends on
> https://lore.kernel.org/linux-arm-msm/20211010030435.4000642-1-bjorn.andersson@linaro.org/
> reducing the connector from a double pointer.
> 
>  drivers/gpu/drm/msm/dp/dp_debug.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c
> b/drivers/gpu/drm/msm/dp/dp_debug.c
> index da4323556ef3..67da4c69eca1 100644
> --- a/drivers/gpu/drm/msm/dp/dp_debug.c
> +++ b/drivers/gpu/drm/msm/dp/dp_debug.c
> @@ -210,26 +210,29 @@ static const struct file_operations 
> test_active_fops = {
>  static int dp_debug_init(struct dp_debug *dp_debug, struct drm_minor 
> *minor)
>  {
>  	int rc = 0;
> +	char path[64];
>  	struct dp_debug_private *debug = container_of(dp_debug,
>  			struct dp_debug_private, dp_debug);
> 
> -	debugfs_create_file("dp_debug", 0444, minor->debugfs_root,
> +	snprintf(path, sizeof(path), "msm_dp-%s", debug->connector->name);
> +
> +	debug->root = debugfs_create_dir(path, minor->debugfs_root);
> +
> +	debugfs_create_file("dp_debug", 0444, debug->root,
>  			debug, &dp_debug_fops);
> 
>  	debugfs_create_file("msm_dp_test_active", 0444,
> -			minor->debugfs_root,
> +			debug->root,
>  			debug, &test_active_fops);
> 
>  	debugfs_create_file("msm_dp_test_data", 0444,
> -			minor->debugfs_root,
> +			debug->root,
>  			debug, &dp_test_data_fops);
> 
>  	debugfs_create_file("msm_dp_test_type", 0444,
> -			minor->debugfs_root,
> +			debug->root,
>  			debug, &dp_test_type_fops);
> 
> -	debug->root = minor->debugfs_root;
> -
>  	return rc;
>  }