From patchwork Sun Sep 16 13:24:43 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ming Lei X-Patchwork-Id: 1463101 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id B44F8DF28C for ; Sun, 16 Sep 2012 13:27:42 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TDEqA-0003NY-US; Sun, 16 Sep 2012 13:24:50 +0000 Received: from mail-ee0-f49.google.com ([74.125.83.49]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TDEq5-0003NK-Tf for linux-arm-kernel@lists.infradead.org; Sun, 16 Sep 2012 13:24:47 +0000 Received: by eekc13 with SMTP id c13so2965893eek.36 for ; Sun, 16 Sep 2012 06:24:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=zawk+O3CFWAZSNfKEVq1BFLPPTL2fmn+Lc5QVW4bAww=; b=nLp/ifkvAcQrWMehFM6W7LZSR69dr0TKAOvkUjvOeoLM6r/bfJEuQUuH/gQG7+Uev+ 2w9w87aJqXRPhjYHHIlqkjFiAH6py8qmzx3set47hnXyEi7J8BpLsMkhV+o5Nmbm0XzB oo9+m/d1UOVa/fbR1xP47qUzZze5a43fG/aLeTIOSbZW45fA3x4Q6tpjo1poCFNbtWJZ UatqjI2o8Ulu2+IZTlB7ZsURZEUmgRdb9iNShjWnTCmBGxsyHkBJVm40sODe17jinXUg O5/uJZyrG9JvfoA2MNS7YOibGv7Zx3+x7QT7Gv0gE5C/yDNmzhh2kyKCXAZK7V+SlJ0s UXfQ== MIME-Version: 1.0 Received: by 10.14.207.9 with SMTP id m9mr10254670eeo.5.1347801883137; Sun, 16 Sep 2012 06:24:43 -0700 (PDT) Received: by 10.14.181.198 with HTTP; Sun, 16 Sep 2012 06:24:43 -0700 (PDT) In-Reply-To: <20120916082510.GN12245@n2100.arm.linux.org.uk> References: <20120818145856.GP18957@n2100.arm.linux.org.uk> <20120916082510.GN12245@n2100.arm.linux.org.uk> Date: Sun, 16 Sep 2012 21:24:43 +0800 Message-ID: Subject: Re: [BUG] Deferred probing in driver model is racy, resulting in lost probes From: Ming Lei To: Russell King - ARM Linux X-Spam-Note: CRM114 invocation failed X-Spam-Score: -2.7 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [74.125.83.49 listed in list.dnswl.org] 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (tom.leiming[at]gmail.com) -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature Cc: Arnd Bergmann , Greg Kroah-Hartman , Mark Brown , linux-kernel@vger.kernel.org, Grant Likely , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On Sun, Sep 16, 2012 at 4:25 PM, Russell King - ARM Linux wrote: > > It isn't. As I said, it's a race condition due to lack of locking - the > driver hasn't been added to the list of drivers at this point: > > int bus_add_driver(struct device_driver *drv) > { > ... > if (drv->bus->p->drivers_autoprobe) { > error = driver_attach(drv); > if (error) > goto out_unregister; > } > klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers); > ... > } > > Notice that the attaching is done _before_ the driver is added to the > bus driver list. Yes, it is a problem since a new device may be added to bus and bus_probe_device() may not see the new added driver. So looks klist_add_tail() should complete before driver_attach() inside bus_add_driver(). The attached one line change should fix the problem because the below way can guarantee that no probe(dev) may be lost. CPU0 CPU1 driver_register ... write(bus->driver_list) smp_mb() read(bus->device_list) ... device_add /* bus_add_device */ write(bus->device_list) smp_mb() /* bus_probe_device*/ read(bus->driver_list) And the smp_mb() has been implicit by UNLOCK+LOCK of 'klist' according to 'VARIETIES OF MEMORY BARRIER' part of Documentation/memory-barriers.txt. Could you test the below patch to see if it can fix your problem? Thanks, diff --git a/drivers/base/bus.c b/drivers/base/bus.c index 181ed26..17d7437 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -714,12 +714,12 @@ int bus_add_driver(struct device_driver *drv) if (error) goto out_unregister; + klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers); if (drv->bus->p->drivers_autoprobe) { error = driver_attach(drv); if (error) goto out_unregister; } - klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers); module_add_driver(drv->owner, drv); error = driver_create_file(drv, &driver_attr_uevent);