From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg KH Subject: Re: [PATCH 3.12-rc7] KVM: Fix modprobe failure for kvm_intel/kvm_amd Date: Wed, 30 Oct 2013 07:23:59 -0700 Message-ID: <20131030142359.GA32196@kroah.com> References: <1383059634-55512-1-git-send-email-tim.gardner@canonical.com> <52700C24.9050001@linux.vnet.ibm.com> <52711121.9030606@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Linus Torvalds , Tim Gardner , Linux Kernel Mailing List , the arch/x86 maintainers , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Gleb Natapov , Marcelo Tosatti , Paolo Bonzini , KVM list , Al Viro To: Raghavendra K T Return-path: Content-Disposition: inline In-Reply-To: <52711121.9030606@linux.vnet.ibm.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org 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 > > 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 > > . 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 > 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 > --- > 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