On Wed, Oct 30, 2013 at 07:31:05PM +0530, Raghavendra K T wrote: > On 10/30/2013 01:03 AM, Linus Torvalds wrote: > > On Tue, Oct 29, 2013 at 12:27 PM, Raghavendra K T > > <raghavendra.kt@xxxxxxxxxxxxxxxxxx> wrote: > >> > >> Could one solution be cascading actual error > >> that is lost in fs/debugfs/inode.c:__create_file(), so that we could > >> take correct action in case of failure of debugfs_create_dir()? > >> > >> (ugly side is we increase total number of params for __create_file to > >> 6). or I hope there could be some better solution. > > > > The solution to this would be to simply return an error-pointer. See > > <linux/err.h>. That's what we do for most complex subsystems that > > return a pointer to a struct: rather than returning "NULL" as an > > error, return the actual error number encoded in the pointer itself. > > Thank you Linus. Yes, this would have been ideal. > > > > > But that would require every user of debugfs_create_dir() to be > > updated to check errors using IS_ERR() instead of checking against > > NULL, and there's quite a few of them. > > > > So I think just making the error be EEXIST is a simpler solution right now. > > > > Agree. > I had below patch, and just before sending as formal mail I saw > Paolo's pull request which already took care of it. > ---8<--- > > > >From ac5d9f038c273f27bea7a54aab6af79b57f56317 Mon Sep 17 00:00:00 2001 > From: Raghavendra K T <raghavendra.kt@xxxxxxxxxxxxxxxxxx> > Date: Wed, 30 Oct 2013 18:59:46 +0530 > Subject: [PATCH] Return EEXIST on debugfs_create_dir failure in kvm > > As quoted by Linus, EFAULT means "user passed in an invalid > virtual address pointer", which is why the error string is Bad address. > But when a debugfs directory creation fails, the above error is not valid. > > Signed-off-by: Raghavendra K T <raghavendra.kt@xxxxxxxxxxxxxxxxxx> > --- > I understand that Tim's patch that renames directory to something like > kvm-pv would solve kvm-amd/kvm-intel modules insertion problem. > This patch is to address error code change complained by Linus. > > arch/x86/kernel/kvm.c | 2 +- > virt/kvm/kvm_main.c | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c > index a0e2a8a..e475fdb 100644 > --- a/arch/x86/kernel/kvm.c > +++ b/arch/x86/kernel/kvm.c > @@ -622,7 +622,7 @@ static int __init kvm_spinlock_debugfs(void) > > d_kvm = kvm_init_debugfs(); > if (d_kvm == NULL) > - return -ENOMEM; > + return -EEXIST; Why even error out at all here? You should never care about debugfs file creation return values, so why pass any error back up the stack? greg k-h -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
