Re: [PATCH] mtd: Fix debugfs file creation when mtd->->dbg.dfs_dir is invalid

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Sat, 11 Nov 2017 15:46:14 +0100
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> Commit e8e3edb95ce6 ("mtd: create per-device and module-scope debugfs
> entries") tried to make MTD related debugfs stuff consistent across the
> MTD framework by creating a root <debugfs>/mtd/ directory containing
> one directory per MTD device.
> 
> The problem is that, by default, the MTD layer only registers the
> master device if no partitions are defined for this master. This
> behavior breaks all drivers that expect mtd->dbg.dfs_dir to be filled
> correctly after calling mtd_device_register() in order to add their own
> debugfs entries.
> 
> The only way we can force all MTD masters to be registered no matter if
> they expose partitions or not is by enabling the
> CONFIG_MTD_PARTITIONED_MASTER option.
> 
> In such situations, there's no other solution but to accept skipping
> debugfs initialization when dbg.dfs_dir is invalid, and when this
> happens, inform the user that he should consider enabling
> CONFIG_MTD_PARTITIONED_MASTER.
> 
> Fixes: e8e3edb95ce6 ("mtd: create per-device and module-scope debugfs entries")
> Cc: <stable@vger.kernel.org>
> Cc: Mario J. Rugiero <mrugiero@gmail.com>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/mtd/devices/docg3.c |  7 ++++++-
>  drivers/mtd/nand/nandsim.c  | 12 +++++++++---
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
> index 84b16133554b..0806f72102c0 100644
> --- a/drivers/mtd/devices/docg3.c
> +++ b/drivers/mtd/devices/docg3.c
> @@ -1814,8 +1814,13 @@ static void __init doc_dbg_register(struct mtd_info *floor)
>  	struct dentry *root = floor->dbg.dfs_dir;
>  	struct docg3 *docg3 = floor->priv;
>  
> -	if (IS_ERR_OR_NULL(root))
> +	if (IS_ERR_OR_NULL(root)) {
> +		if (IS_ENABLED(CONFIG_DEBUG_FS) &&
> +		    !IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER))
> +			dev_warn(floor->dev.parent,
> +				 "CONFIG_MTD_PARTITIONED_MASTER must be enabled to expose debugfs stuff\n");
>  		return;
> +	}
>  
>  	debugfs_create_file("docg3_flashcontrol", S_IRUSR, root, docg3,
>  			    &flashcontrol_fops);
> diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
> index 246b4393118e..a22f4d7ca1cb 100644
> --- a/drivers/mtd/nand/nandsim.c
> +++ b/drivers/mtd/nand/nandsim.c
> @@ -520,11 +520,17 @@ static int nandsim_debugfs_create(struct nandsim *dev)
>  	struct dentry *root = nsmtd->dbg.dfs_dir;
>  	struct dentry *dent;
>  
> -	if (!IS_ENABLED(CONFIG_DEBUG_FS))
> -		return 0;
> +	if (IS_ENABLED(CONFIG_DEBUG_FS) &&
> +	    !IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) &&
> +	    dev->nbparts)
> +		NS_WARN("CONFIG_MTD_PARTITIONED_MASTER must be enabled to expose debugfs stuff\n");
>  

Actually we can't rely on dev->nbparts here because partitions can be
defined through the cmdline (using mtdpard=).

I'll fix that with something similar to what is done in the docg3 driver:

	/*
	 * Just skip debugfs initialization when the debugfs directory is
	 * missing.
	 */
  	if (IS_ERR_OR_NULL(root)) {
		if (IS_ENABLED(CONFIG_DEBUG_FS) &&
		    !IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER))
			NS_WARN("CONFIG_MTD_PARTITIONED_MASTER must be enabled to expose debugfs stuff\n");
		return 0;
	}

> +	/*
> +	 * Just skip debugfs initialization when the debugfs directory is
> +	 * missing.
> +	 */
>  	if (IS_ERR_OR_NULL(root))
> -		return -1;
> +		return 0;
>  
>  	dent = debugfs_create_file("nandsim_wear_report", S_IRUSR,
>  				   root, dev, &dfs_fops);