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 07:46:37PM +0200, Greg Kroah-Hartman wrote:
> 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 :)

Oh, forgot the pointer to the tree:
	https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/log/?h=debugfs_cleanup
this is all public, just not in linux-next as it's being fed through 50+
different subsystem trees.

thanks,

greg k-h

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