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

Greg KH
2023-02-02T08:30:43Z

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