Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751939AbbBRGKM (ORCPT ); Wed, 18 Feb 2015 01:10:12 -0500 Received: from relay1.mentorg.com ([192.94.38.131]:39794 "EHLO relay1.mentorg.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750845AbbBRGKK (ORCPT ); Wed, 18 Feb 2015 01:10:10 -0500 Message-ID: <54E42CBA.5010007@mentor.com> Date: Wed, 18 Feb 2015 11:40:02 +0530 From: Harish Jenny Kandiga Nagaraj User-Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Rusty Russell , Lucas De Marchi CC: linux-modules , lkml Subject: Re: [PATCH] libkmod-module: Remove directory existence check for KMOD_MODULE_BUILTIN References: <1424177796-17923-1-git-send-email-harish_kandiga@mentor.com> <874mqjuaky.fsf@rustcorp.com.au> In-Reply-To: <874mqjuaky.fsf@rustcorp.com.au> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [137.202.0.76] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2953 Lines: 60 On Wednesday 18 February 2015 09:37 AM, Rusty Russell wrote: > Lucas De Marchi writes: >> 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? > Not deliberately. > >> Or is the creation of /sys/module/ and >> /sys/module//initstate not atomic? > No, it's not atomic :( That makes it unreliable (it seemed like a good > idea in 2006 I guess). > > Here's what happens from a kernel side: > > 1) Module is marked UNFORMED. > 2) Check there's no module by same name already. If there is, and it's > UNFORMED or COMING, we wait. > 3) module is marked COMING > 4) module parses arguments. > 5) sysfs directory and attributes are created. > 6) module's init is called. > 7) module is marked LIVE. These are the operations handled in kernel after syscall/init_module is called. Here is the list of operations which happen before point number 1) 0a) __request_module/try_then_request_module gets called from kernel. 0b) call_modprobe gets called which calls usermode modprobe to see if module is loaded. 0c) syscall init_module gets called from modprobe. > So, the second init_module should be waiting. > > I tested this by putting a sleep in the nls_cp437 init, and watching > what modprobe did. It works correctly. Problem here is second init_module is not yet called. if it gets called , it is handled properly. Adding a sleep in nls_cp437 is handled properly. We need to add sleep in module_add_modinfo_attrs ( module.c ) while creating sysfs files(sysfs_create_file) in order to reproduce the issue I mentioned. > > You are checking initstate, and getting caught in the race: > > 783 14:33:14.170205 open("/sys/module/nls_cp437/initstate", O_RDONLY|O_LARGEFILE|O_CLOEXEC) > > You should probably check initstate *after* loading fails. It's > possible that it's unloaded in the meantime, but the race is pretty > narrow and unloading is unusual. Did not get the point of checking initstate after loading fails. However we need to handle even unusual rare cases as well. > > Cheers, > Rusty. -- 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/