From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH 3.12-rc7] KVM: Fix modprobe failure for kvm_intel/kvm_amd Date: Wed, 30 Oct 2013 17:40:46 +0100 Message-ID: <5271368E.6060802@redhat.com> References: <1383059634-55512-1-git-send-email-tim.gardner@canonical.com> <52700C24.9050001@linux.vnet.ibm.com> <52711121.9030606@linux.vnet.ibm.com> <20131030142359.GA32196@kroah.com> <5271284A.6000002@linux.vnet.ibm.com> <527129D4.3010308@redhat.com> <20131030155930.GA23433@kroah.com> <52712EE9.9050402@redhat.com> <20131030162342.GA21211@kroah.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Raghavendra K T , Linus Torvalds , Tim Gardner , Linux Kernel Mailing List , the arch/x86 maintainers , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Gleb Natapov , Marcelo Tosatti , KVM list , Al Viro To: Greg KH Return-path: In-Reply-To: <20131030162342.GA21211@kroah.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org Il 30/10/2013 17:23, Greg KH ha scritto: >> > static inline struct dentry *debugfs_create_dir(const char *name, >> > struct dentry *parent) >> > { >> > return ERR_PTR(-ENODEV); >> > } >> > >> > which would oops a lot of the current callers. > It will oops? Really? Where? That shouldn't happen at all. Doh, it obviously won't because the bogus pointer value will never be dereferenced by the dummy debugfs_create_file. >> > Very few places use the currently correct idiom >> > >> > if (IS_ERR(root) || !root) >> > >> > but it's very ugly... Perhaps debugfs_create_dir *should* return an >> > error-valued pointer after all. > Or just don't care about the return value, and all will work out just > fine, right? Debugfs files would then be created in the debugfs root, which is not nice. /* If the parent is not specified, we create it in the root. * We need the root dentry to do this, which is in the super * block. A pointer to that is in the struct vfsmount that we * have around. */ if (!parent) parent = debugfs_mount->mnt_root; This is all documented right: /** * debugfs_create_file - create a file in the debugfs filesystem * ... * This function will return a pointer to a dentry if it succeeds. This * pointer must be passed to the debugfs_remove() function when the file is * to be removed (no automatic cleanup happens if your module is unloaded, * you are responsible here.) If an error occurs, %NULL will be returned. * * If debugfs is not enabled in the kernel, the value -%ENODEV will be * returned. */ so I guess it's just part of the API. Paolo