From patchwork Tue Jan 26 17:59:13 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Jon Medhurst \(Tixy\)" X-Patchwork-Id: 60502 Delivered-To: patch@linaro.org Received: by 10.112.130.2 with SMTP id oa2csp2116479lbb; Tue, 26 Jan 2016 10:00:46 -0800 (PST) X-Received: by 10.98.69.209 with SMTP id n78mr35089547pfi.81.1453831246213; Tue, 26 Jan 2016 10:00:46 -0800 (PST) Return-Path: Received: from bombadil.infradead.org (bombadil.infradead.org. [2001:1868:205::9]) by mx.google.com with ESMTPS id o28si700265pfa.151.2016.01.26.10.00.45 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 26 Jan 2016 10:00:46 -0800 (PST) Received-SPF: pass (google.com: domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) client-ip=2001:1868:205::9; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) smtp.mailfrom=linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aO7u5-0000Xu-F1; Tue, 26 Jan 2016 17:59:45 +0000 Received: from smarthost03b.mail.zen.net.uk ([212.23.1.21]) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aO7tz-0000QE-RL for linux-arm-kernel@lists.infradead.org; Tue, 26 Jan 2016 17:59:41 +0000 Received: from [82.69.122.217] (helo=[192.168.2.110]) by smarthost03b.mail.zen.net.uk with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1aO7tZ-0003f8-T3; Tue, 26 Jan 2016 17:59:14 +0000 Message-ID: <1453831153.2850.107.camel@linaro.org> Subject: Problem with component helpers and probe deferral in 4.5-rc1 From: "Jon Medhurst (Tixy)" To: Russell King - ARM Linux Date: Tue, 26 Jan 2016 17:59:13 +0000 X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 X-Originating-smarthost03b-IP: [82.69.122.217] X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160126_095940_060827_A9338A26 X-CRM114-Status: GOOD ( 23.48 ) X-Spam-Score: 0.6 (/) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (0.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [212.23.1.21 listed in list.dnswl.org] -0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [212.23.1.21 listed in wl.mailspike.net] 2.5 SUSPICIOUS_RECIPS Similar addresses in recipient list 0.7 SPF_SOFTFAIL SPF: sender does not match SPF record (softfail) -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Greg Kroah-Hartman , Liviu Dudau , linux-arm-kernel@lists.infradead.org, dri-devel@lists.freedesktop.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org I believe I've found a problem with the component helpers and/or how drivers use them. I discovered this whilst trying to get ARM's HDLCD driver [1] working on 4.5-rc1, however I believe that code is following a pattern used by drivers already in 4.5 and the problem isn't specific to it. This is what I have observed... The master device's probe function uses component_match_add to create a match list then it passes this to component_master_add_with_match. That creates a struct master and then calls try_to_bring_up_master which: - Calls find_components to attach all components to the master. - Calls master->ops->bind() If this bind call fails (with -EPROBE_DEFER due to missing clock in my case) then this error is returned to component_master_add_with_match which proceeds to delete the master struct. However, find_components has already attached components to that deleted master, so I think we also need something to detach components as well. I've added a patch at the end of this email which does that directly, but I wonder if instead it's the responsibility of the driver for the master to call component_master_del on error? Finally, with my scenario which has probe deferring, some time later the original master device will be probed again, repeating all the above. Except that now find_components doesn't find the components because they are already attached to a different master (the old master struct which was deleted) this results in a permanent error. Which is what lead me to investigate. [1] https://lkml.org/lkml/2015/12/22/451 The patch to detach components when master is deleted... ------------------------------------------------------------------------- From: Jon Medhurst Subject: [PATCH] component: Detach components when deleting master struct component_master_add_with_match calls find_components which, if any components already exist, it attaches to the master struct. However, if we later encounter an error the master struct is deleted, leaving components with a dangling pointer to it. If the error was a temporary one, e.g. for probe deferral, then when the master device is re-probed, it will fail to find the required components as they appear to already be attached to a master. Fix this by nulling components pointers to the master struct when it is deleted. This code is factored out into a separate function so it can be shared with component_master_del. Signed-off-by: Jon Medhurst --- drivers/base/component.c | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) -- 2.1.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel diff --git a/drivers/base/component.c b/drivers/base/component.c index 89f5cf68..a3a1394 100644 --- a/drivers/base/component.c +++ b/drivers/base/component.c @@ -283,6 +283,24 @@ void component_match_add_release(struct device *master, } EXPORT_SYMBOL(component_match_add_release); +static void free_master(struct master *master) +{ + struct component_match *match = master->match; + int i; + + list_del(&master->node); + + if (match) { + for (i = 0; i < match->num; i++) { + struct component *c = match->compare[i].component; + if (c) + c->master = NULL; + } + } + + kfree(master); +} + int component_master_add_with_match(struct device *dev, const struct component_master_ops *ops, struct component_match *match) @@ -309,11 +327,9 @@ int component_master_add_with_match(struct device *dev, ret = try_to_bring_up_master(master, NULL); - if (ret < 0) { - /* Delete off the list if we weren't successful */ - list_del(&master->node); - kfree(master); - } + if (ret < 0) + free_master(master); + mutex_unlock(&component_mutex); return ret < 0 ? ret : 0; @@ -324,25 +340,12 @@ void component_master_del(struct device *dev, const struct component_master_ops *ops) { struct master *master; - int i; mutex_lock(&component_mutex); master = __master_find(dev, ops); if (master) { - struct component_match *match = master->match; - take_down_master(master); - - list_del(&master->node); - - if (match) { - for (i = 0; i < match->num; i++) { - struct component *c = match->compare[i].component; - if (c) - c->master = NULL; - } - } - kfree(master); + free_master(master); } mutex_unlock(&component_mutex); }