Re: [PATCH] soc: ti: knav_dma: remove debugfs file on teardown

[Hari Prasath]

Hello,

On 15/06/26 2:42 pm, Pengpeng Hou wrote:
> knav_dma_probe() creates the global knav_dma debugfs file whose show
> callback walks the global kdev list. knav_dma_remove() tears down the DMA
> instances and runtime PM but leaves that debugfs file and global ready
> state behind.
>
> Save the debugfs dentry, remove it before destroying the DMA state, and
> clear the global ready pointer state during remove.
>
> Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>
> ---
>   drivers/soc/ti/knav_dma.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/ti/knav_dma.c b/drivers/soc/ti/knav_dma.c
> index e5f5e3142fc4..9277c525ac21 100644
> --- a/drivers/soc/ti/knav_dma.c
> +++ b/drivers/soc/ti/knav_dma.c
> @@ -125,6 +125,7 @@ struct knav_dma_chan {
>   			ch->channel : ch->flow)
>   
>   static struct knav_dma_pool_device *kdev;
> +static struct dentry *knav_dma_debugfs;
>   
>   static bool device_ready;
>   bool knav_dma_device_ready(void)
> @@ -740,8 +741,9 @@ static int knav_dma_probe(struct platform_device *pdev)
>   		goto err_put_sync;
>   	}
>   
> -	debugfs_create_file("knav_dma", S_IFREG | S_IRUGO, NULL, NULL,
> -			    &knav_dma_debug_fops);
> +	knav_dma_debugfs = debugfs_create_file("knav_dma", 0444,
> +					       NULL, NULL,
> +					       &knav_dma_debug_fops);
>   
>   	device_ready = true;
>   	return ret;
> @@ -758,6 +760,10 @@ static void knav_dma_remove(struct platform_device *pdev)
>   {
>   	struct knav_dma_device *dma;
>   
> +	device_ready = false;
> +	debugfs_remove(knav_dma_debugfs);

Does this handle the case where a process already has the debugfs file 
open? A concurrent read() from the userspace could still potentially 
reach knav_dma_debug_show() and access kdev while it's being torn down
below. Pls check and add necessary protection around kdev access.

Regards,
Hari 


> +	knav_dma_debugfs = NULL;
> +
>   	list_for_each_entry(dma, &kdev->list, list) {
>   		if (atomic_dec_return(&dma->ref_count) == 0)
>   			knav_dma_hw_destroy(dma);
> @@ -765,6 +771,7 @@ static void knav_dma_remove(struct platform_device *pdev)
>   
>   	pm_runtime_put_sync(&pdev->dev);
>   	pm_runtime_disable(&pdev->dev);
> +	kdev = NULL;
>   }
>   
>   static struct of_device_id of_match[] = {