Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753599AbbBSBTh (ORCPT <rfc822;w@1wt.eu>);
	Wed, 18 Feb 2015 20:19:37 -0500
Received: from mail-ob0-f180.google.com ([209.85.214.180]:39027 "EHLO
	mail-ob0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752136AbbBSBTf (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 18 Feb 2015 20:19:35 -0500
MIME-Version: 1.0
In-Reply-To: <87vbiysv1v.fsf@rustcorp.com.au>
References: <1424177796-17923-1-git-send-email-harish_kandiga@mentor.com>
 <CAKi4VA+nJknEwkQfUWf0VbcEMzzz3uHwwNe8OuWQOi-K0sWxjQ@mail.gmail.com>
 <874mqjuaky.fsf@rustcorp.com.au> <CAKi4VAKpzUcakX+BY8dYE8Jx4BUOqy_-LZWH5gJWtwn3EdbE0g@mail.gmail.com>
 <87vbiysv1v.fsf@rustcorp.com.au>
From: Lucas De Marchi <lucas.de.marchi@gmail.com>
Date: Wed, 18 Feb 2015 23:19:14 -0200
Message-ID: <CAKi4VAKc_zrPptt2ZKT_Jj8bLdL1bt0emhtEuodV2WKkas9ZOw@mail.gmail.com>
Subject: Re: [PATCH] libkmod-module: Remove directory existence check for KMOD_MODULE_BUILTIN
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: Harish Jenny K N <harish_kandiga@mentor.com>,
        linux-modules <linux-modules@vger.kernel.org>,
        lkml <linux-kernel@vger.kernel.org>,
        greg KH <gregkh@linuxfoundation.org>
Content-Type: text/plain; charset=UTF-8
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2306
Lines: 54

On Wed, Feb 18, 2015 at 8:40 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Lucas De Marchi <lucas.de.marchi@gmail.com> writes:
>> On Wed, Feb 18, 2015 at 2:07 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Yeah, I just thought (an wanted that) the attributes were being
>> created first and then hooked up in the sysfs tree under
>> /sys/module/<modulename>. I.e. if the directory exists and there's no
>> initstate this is because it's a builtin module. I don't want to
>> wait/sleep on the file to appear because users of
>> kmod_module_get_initstate() may not tolerate this behavior.
>>
>> Looking up at the old module-init-tools, it used an ugly loop with
>> usleep() before trying to read the file again :-/
>>
>> Can we change kernel side guaranteeing the initstate file appears
>> together with the directory?
>
> Greg?  The core problem is that kmod looks for
> /sys/module/<name>/initstate; if it's not there, it assumes a builtin
> module.

Just to make it clear:

We try to open /sys/module/<name>/initstate. If it fails we stat
/sys/module/<name> checking if it exists and is a directory. If it
does then we assume the module is builtin.

> However, this is racy when a module is being inserted.  Is there a way
> to create this sysfs file and dir atomically?

Greg, the question is still valid since it'd be nice to have this
guarantee and be able to correctly reply the state with whatever is in
initstate file, but...

Rusty, thinking again if we fallback to "coming" instead of "builtin"
everything should be fine, no? Because the decision about builtin has
already been taken by looking at the modules.builtin index. If we
return "coming" here the second call to modprobe would call
init_module() again which would wait for the first one to complete (or
return EEXIST if it's already live) since we only shortcut the
init_module() call if the module is live or builtin

what do you think?


Harrish, in your patch if you just change the "return
KMOD_MODULE_BUILTIN;" to "return KMOD_MODULE_COMING;" does it work?

-- 
Lucas De Marchi
--
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/