Re: [PATCH -next 2/3] devtmpfs: add debug info to handle()

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Feb 02, 2023 at 03:32:02AM +0000, Longlong Xia wrote:
> The devtmpfs_*_node() are used to mount/unmount devices to /dev, but their
> callers don't check their return value, so we don't know the reason for
> the failure. Let's add some debug info in handle() to help users know
> why failed.
> 
> Signed-off-by: Longlong Xia <xialonglong1@huawei.com>
> ---
>  drivers/base/devtmpfs.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
> index ae72d4ba8547..77ca64f708ce 100644
> --- a/drivers/base/devtmpfs.c
> +++ b/drivers/base/devtmpfs.c
> @@ -389,10 +389,18 @@ static __initdata DECLARE_COMPLETION(setup_done);
>  static int handle(const char *name, umode_t mode, kuid_t uid, kgid_t gid,
>  		  struct device *dev)
>  {
> +	int ret;
> +
>  	if (mode)
> -		return handle_create(name, mode, uid, gid, dev);
> +		ret = handle_create(name, mode, uid, gid, dev);
>  	else
> -		return handle_remove(name, dev);
> +		ret = handle_remove(name, dev);
> +
> +	if (ret)
> +		pr_err_ratelimited("failed to %s %s, ret = %d\n",
> +				   mode ? "create" : "remove", name, ret);

As you have a struct device * here, why not use dev_err() instead?

And why rate limited?  What is going to cause this to be spammed with
lots and lots of failures?  What were you doing that caused a failure
here, how is it triggered?

thanks,

greg k-h