Re: [PATCH] coresight: cpu-debug: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Jun 18, 2019 at 11:23:25AM -0600, Mathieu Poirier wrote:
> Hi Greg,
> 
> On Tue, 18 Jun 2019 at 09:52, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > When calling debugfs functions, there is no need to ever check the
> > return value.  The function can work or not, but the code logic should
> > never do something different based on this.
> 
> Looking around in the kernel there is no shortage of instances where
> the return value of debugfs functions are checked and the logic
> altered based on these values.  But there are also just as many that
> don't...  It also seems counter intuitive to ignore the return value
> of any function, something that in most case is guaranteed to raise
> admonition.

In my tree, those instances are almost all gone.  I've also posted over
100+ patches in the past few weeks to clean this up.

> That being said I am sure there is a good reason to support your
> position - would you mind expanding a little so that I can follow?

No kernel code should ever care if debugfs works or not.  No user code
should ever require it for normal operation either.  debugfs was written
to be simple and easy to use, no need to check any return values at all.

Any return value of a debugfs call can be fed back into another call
with no issues at all.

Also, due to some debugfs core changes a few kernel releases ago, the
checks:
	if (!debug_debugfs_dir) {
...
	if (!file) {
can never trigger as debugfs_create_dir() or debugfs_create_file() can
never return NULL (and in the past, it almost never would either).  So
as it is, that code isn't correct anyway (my fault, I know, hey, I'm
trying to fix it!)

I'm trying to make things simple, and easy, and impossible to get wrong.
I know it goes against the normal "robust" kernel development mentality,
but there is no need to ever care about debugfs at all.

The reason I started all of this is that we have found places where
userspace, and the kernel, was depending on the proper operation of
debugfs.  In one horrid example, a device would not display the batter
level if debugfs was disabled.  In another case, the kernel was actually
relying on a debugfs call to fail in order to handle some logic the
subsystem should have been doing on its own.  All of that has now been
cleaned up, and I am working on making debugfs just not return any
values at all to prevent this type of mess happening again.

And hey, I am removing code, here's my current tree as a diff from
what is not already merged into linux-next:
	 301 files changed, 1394 insertions(+), 4637 deletions(-)
that's always a good thing :)

Hopefully this helps explain things better.

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel