On Tue, Jan 22, 2019 at 05:33:37PM +0100, Greg Kroah-Hartman wrote: > On Tue, Jan 22, 2019 at 05:24:54PM +0100, Heiko Carstens wrote: > > On Tue, Jan 22, 2019 at 04:21:02PM +0100, Greg Kroah-Hartman 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. > > > > > > Cc: Martin Schwidefsky <schwidefsky@xxxxxxxxxx> > > > Cc: Heiko Carstens <heiko.carstens@xxxxxxxxxx> > > > Cc: Kees Cook <keescook@xxxxxxxxxxxx> > > > Cc: Christian Borntraeger <borntraeger@xxxxxxxxxx> > > > Cc: Hendrik Brueckner <brueckner@xxxxxxxxxxxxxxxxxx> > > > Cc: linux-s390@xxxxxxxxxxxxxxx > > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > > --- > > > arch/s390/kernel/debug.c | 6 ------ > > > arch/s390/kernel/kdebugfs.c | 2 -- > > > arch/s390/kernel/sysinfo.c | 2 -- > > > 3 files changed, 10 deletions(-) > > > diff --git a/arch/s390/kernel/sysinfo.c b/arch/s390/kernel/sysinfo.c > > > index 12f80d1f0415..2ac3c9b56a13 100644 > > > --- a/arch/s390/kernel/sysinfo.c > > > +++ b/arch/s390/kernel/sysinfo.c > > > @@ -545,8 +545,6 @@ static __init int stsi_init_debugfs(void) > > > int lvl, i; > > > > > > stsi_root = debugfs_create_dir("stsi", arch_debugfs_dir); > > > - if (IS_ERR_OR_NULL(stsi_root)) > > > - return 0; > > > > No objections, however will you also change the odd behaviour that > > e.g. debugfs_create_file() returns -ENODEV instead of (the expected) > > NULL pointer if CONFIG_DEBUGFS is disabled? > > Nope. That is intentional. > > > I do remember this since it caused at least one crash ;) > > Which is why you shouldn't care about the return value of these > functions :) > > > 19cdd08ba155 ("[S390] qdio: fix broken pointer in case of CONFIG_DEBUG_FS is disabled"). > > Odd, what crashes when passed an error pointer? What was someone trying > to do with those pointers? The only thing you can do with a return > value from a debugfs function is to pass it back into another debugfs > call. Sounds like someone wasn't doing that :( I think it used to be this code that crashed: static void remove_debugfs_entry(struct qdio_q *q) { int i; for (i = 0; i < MAX_DEBUGFS_QUEUES; i++) { if (!debugfs_queues[i]) continue; -----> if (debugfs_queues[i]->d_inode->i_private == q) { debugfs_remove(debugfs_queues[i]); debugfs_queues[i] = NULL; } } } Which looks like a layering violation anyway. However this code is gone, so everything should be fine.