From: Atul Kumar Pant <atulpant.linux@gmail.com>
To: Michal Simek <michal.simek@amd.com>
Cc: shubhrajyoti.datta@amd.com, sai.krishna.potthuri@amd.com,
bp@alien8.de, tony.luck@intel.com, james.morse@arm.com,
mchehab@kernel.org, rric@kernel.org, linux-edac@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, shuah@kernel.org
Subject: Re: [PATCH v1] drivers: edac: Drop unnecessary error check for debugfs_create_dir
Date: Mon, 28 Aug 2023 19:05:47 +0530 [thread overview]
Message-ID: <20230828133547.GA58271@atom0118> (raw)
In-Reply-To: <723e803b-6f8b-ceb3-e987-4a6f83d89222@amd.com>
On Fri, Aug 25, 2023 at 09:31:54AM +0200, Michal Simek wrote:
>
>
> On 8/15/23 22:38, Atul Kumar Pant wrote:
> > This patch removes the error checking for debugfs_create_dir.
>
> Avoid using "This patch".
Thanks for pointing this out. I'll remember this.
>
> > Even if we get an error from this function, other debugfs APIs will
> > handle the error value and doesn't crash in that case. Hence caller can
> > safely ignore the errors that occur during the creation of debugfs nodes.
>
> First of all which issue do you have? Did you see that folder is not created?
I have not seen any issue as such. But going by the comments before
the debugfs_create_dir API (https://elixir.bootlin.com/linux/latest/source/fs/debugfs/inode.c#L583),
we can ignore safely ignore the return value from this API.
>
> I am not quite sure if this is the right behavior.
> In the code there is
> 135 if (!parent)
> 136 parent = edac_debugfs;
>
> It means you are right that if creating ocm folder can fail and properties
> will be still created under edac_debugfs but is this the right behavior?
>
> altera_edac/armada_xp_edac/i10nm/i5100/igen6/others are checking return
> value that's why I can't see any reason to remove this checking from one
> driver.
>
> If you want to fix all please send patch for all but I don't think it will
> improve situation and it will just hide different issue if creating folder
> fails.
Understood your point. Are you suggesting that we should keep these
checks as it is, or should I fix for all the drivers and upload the
patch ?
> And debugfs will be disabled in production system anyway.
>
> Thanks,
> Michal
>
>
>
>
WARNING: multiple messages have this Message-ID (diff)
From: Atul Kumar Pant <atulpant.linux@gmail.com>
To: Michal Simek <michal.simek@amd.com>
Cc: shubhrajyoti.datta@amd.com, sai.krishna.potthuri@amd.com,
bp@alien8.de, tony.luck@intel.com, james.morse@arm.com,
mchehab@kernel.org, rric@kernel.org, linux-edac@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, shuah@kernel.org
Subject: Re: [PATCH v1] drivers: edac: Drop unnecessary error check for debugfs_create_dir
Date: Mon, 28 Aug 2023 19:05:47 +0530 [thread overview]
Message-ID: <20230828133547.GA58271@atom0118> (raw)
In-Reply-To: <723e803b-6f8b-ceb3-e987-4a6f83d89222@amd.com>
On Fri, Aug 25, 2023 at 09:31:54AM +0200, Michal Simek wrote:
>
>
> On 8/15/23 22:38, Atul Kumar Pant wrote:
> > This patch removes the error checking for debugfs_create_dir.
>
> Avoid using "This patch".
Thanks for pointing this out. I'll remember this.
>
> > Even if we get an error from this function, other debugfs APIs will
> > handle the error value and doesn't crash in that case. Hence caller can
> > safely ignore the errors that occur during the creation of debugfs nodes.
>
> First of all which issue do you have? Did you see that folder is not created?
I have not seen any issue as such. But going by the comments before
the debugfs_create_dir API (https://elixir.bootlin.com/linux/latest/source/fs/debugfs/inode.c#L583),
we can ignore safely ignore the return value from this API.
>
> I am not quite sure if this is the right behavior.
> In the code there is
> 135 if (!parent)
> 136 parent = edac_debugfs;
>
> It means you are right that if creating ocm folder can fail and properties
> will be still created under edac_debugfs but is this the right behavior?
>
> altera_edac/armada_xp_edac/i10nm/i5100/igen6/others are checking return
> value that's why I can't see any reason to remove this checking from one
> driver.
>
> If you want to fix all please send patch for all but I don't think it will
> improve situation and it will just hide different issue if creating folder
> fails.
Understood your point. Are you suggesting that we should keep these
checks as it is, or should I fix for all the drivers and upload the
patch ?
> And debugfs will be disabled in production system anyway.
>
> Thanks,
> Michal
>
>
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-08-28 13:37 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-15 20:38 [PATCH v1] drivers: edac: Drop unnecessary error check for debugfs_create_dir Atul Kumar Pant
2023-08-15 20:38 ` Atul Kumar Pant
2023-08-25 7:31 ` Michal Simek
2023-08-25 7:31 ` Michal Simek
2023-08-28 13:35 ` Atul Kumar Pant [this message]
2023-08-28 13:35 ` Atul Kumar Pant
2023-08-28 14:00 ` Michal Simek
2023-08-28 14:00 ` Michal Simek
2023-08-31 4:20 ` Atul Kumar Pant
2023-08-31 4:20 ` Atul Kumar Pant
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230828133547.GA58271@atom0118 \
--to=atulpant.linux@gmail.com \
--cc=bp@alien8.de \
--cc=james.morse@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-edac@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=michal.simek@amd.com \
--cc=rric@kernel.org \
--cc=sai.krishna.potthuri@amd.com \
--cc=shuah@kernel.org \
--cc=shubhrajyoti.datta@amd.com \
--cc=tony.luck@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.