Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752596AbbBQRbV (ORCPT ); Tue, 17 Feb 2015 12:31:21 -0500 Received: from mail-ob0-f170.google.com ([209.85.214.170]:42390 "EHLO mail-ob0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752444AbbBQRbT (ORCPT ); Tue, 17 Feb 2015 12:31:19 -0500 MIME-Version: 1.0 In-Reply-To: <1424177796-17923-1-git-send-email-harish_kandiga@mentor.com> References: <1424177796-17923-1-git-send-email-harish_kandiga@mentor.com> From: Lucas De Marchi Date: Tue, 17 Feb 2015 15:30:58 -0200 Message-ID: Subject: Re: [PATCH] libkmod-module: Remove directory existence check for KMOD_MODULE_BUILTIN To: Harish Jenny K N , Rusty Russell Cc: linux-modules , lkml Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3706 Lines: 91 On Tue, Feb 17, 2015 at 10:56 AM, Harish Jenny K N wrote: > usecase: two sd cards are being mounted in parallel at same time on > dual core. example modules which are getting loaded is nls_cp437. > While one module is being loaded , it starts creating sysfs files. > meanwhile on other core, modprobe might return saying the module > is KMOD_MODULE_BUILTIN, which might result in not mounting sd card. an {f,}init_module() call should not return until the sysfs files are created and if there is /sys/module// there should be /sys/module//initstate unless the module is builtin. Rusty, was there any changes in this area in the kernel recently? Or is the creation of /sys/module/ and /sys/module//initstate not atomic? See patch below. -- Lucas De Marchi > > Experiments done to prove the issue in kmod. > Added sleep in kernel module.c at the place of creation of sysfs files. > Then tried `modprobe nls_cp437` from two different shells. > While the first was still waiting for its completion , > the second one returned saying the module is built-in. > > built-in modules are handled by searching the modules.builtin file. > mod->builtin gets set and are handled in kmod_module_get_initstate function. > Removing the checking of the presence of /sys/module// > directory, which may not be required. It has to be added in other place > accordingly if required. > > Signed-off-by: Harish Jenny K N > --- > libkmod/libkmod-module.c | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) > > diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c > index 30f15ca..21c2a7e 100644 > --- a/libkmod/libkmod-module.c > +++ b/libkmod/libkmod-module.c > @@ -1708,7 +1708,7 @@ KMOD_EXPORT const char *kmod_module_initstate_str(enum kmod_module_initstate sta > KMOD_EXPORT int kmod_module_get_initstate(const struct kmod_module *mod) > { > char path[PATH_MAX], buf[32]; > - int fd, err, pathlen; > + int fd, err; > > if (mod == NULL) > return -ENOENT; > @@ -1716,7 +1716,7 @@ KMOD_EXPORT int kmod_module_get_initstate(const struct kmod_module *mod) > if (mod->builtin) > return KMOD_MODULE_BUILTIN; > > - pathlen = snprintf(path, sizeof(path), > + snprintf(path, sizeof(path), > "/sys/module/%s/initstate", mod->name); > fd = open(path, O_RDONLY|O_CLOEXEC); > if (fd < 0) { > @@ -1725,15 +1725,6 @@ KMOD_EXPORT int kmod_module_get_initstate(const struct kmod_module *mod) > DBG(mod->ctx, "could not open '%s': %s\n", > path, strerror(-err)); > > - if (pathlen > (int)sizeof("/initstate") - 1) { > - struct stat st; > - path[pathlen - (sizeof("/initstate") - 1)] = '\0'; > - if (stat(path, &st) == 0 && S_ISDIR(st.st_mode)) > - return KMOD_MODULE_BUILTIN; > - } > - > - DBG(mod->ctx, "could not open '%s': %s\n", > - path, strerror(-err)); > return err; > } > > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-modules" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/