From patchwork Mon Jan 19 18:55:56 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wolfram Sang X-Patchwork-Id: 430644 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id B755514019D for ; Tue, 20 Jan 2015 05:57:13 +1100 (AEDT) Received: from ozlabs.org (ozlabs.org [103.22.144.67]) by lists.ozlabs.org (Postfix) with ESMTP id 7AC9E1A0D92 for ; Tue, 20 Jan 2015 05:57:13 +1100 (AEDT) X-Original-To: linuxppc-dev@lists.ozlabs.org Delivered-To: linuxppc-dev@lists.ozlabs.org Received: from pokefinder.org (sauhun.de [89.238.76.85]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id B58B91A0D01 for ; Tue, 20 Jan 2015 05:56:33 +1100 (AEDT) Received: from p4fe24a52.dip0.t-ipconnect.de ([79.226.74.82]:60436 helo=localhost) by pokefinder.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1YDHUv-0001uQ-1c; Mon, 19 Jan 2015 19:56:25 +0100 From: Wolfram Sang To: linux-kernel@vger.kernel.org Subject: [PATCH] i2c: drop ancient protection against sysfs refcounting issues Date: Mon, 19 Jan 2015 19:55:56 +0100 Message-Id: <1421693756-12917-1-git-send-email-wsa@the-dreams.de> X-Mailer: git-send-email 2.1.3 Cc: linux-mips@linux-mips.org, Wolfram Sang , Greg Kroah-Hartman , Pantelis Antoniou , Julia Lawall , linux-arm-kernel@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, Jean Delvare X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Sender: "Linuxppc-dev" Back in the days, sysfs seemed to have refcounting issues and subsystems needed a completion to be safe. This is not the case anymore, so I2C can get rid of this code. There is noone else besides I2C doing something like this currently (checked with the attached coccinelle script which checks if a release function exists and if it contains a completion). I have been digging through the history of linux.git and linux-history.git and found that e.g. w1 used to have such a mechanism and also simply removed it later. Some more info from Greg Kroah-Hartman: "Having that call "wait" for the other release call to happen is really old, as Jean points out, from 2003. We have "fixed" sysfs since then to detach the files from the devices easier, we used to have some nasy reference count issues in that area." And some testing from Jean Delvare which matches my results: "However I just tested unloading an i2c bus driver while its adapter's new_device attribute was opened and rmmod returned immediately. So it doesn't look like accessing sysfs attributes actually takes a reference to the underlying i2c_adapter." Let's get rid of this code before really nobody knows/understands anymore what this was for and if it has a subtle use. Reported-by: Pantelis Antoniou Signed-off-by: Wolfram Sang Cc: Greg Kroah-Hartman Cc: Jean Delvare Cc: Julia Lawall --- Of course, more testing is appreciated. Here is the coccinelle script: === @has_type@ identifier d_type, rel_f; @@ struct device_type d_type = { .release = rel_f, }; @has_device@ struct device *d; identifier rel_f, p; @@ ( p->dev.release = &rel_f; | d->release = &rel_f; ) @find_type depends on has_type@ identifier has_type.rel_f, d; @@ void rel_f(struct device *d) { ... * complete(...); ... } @find_device depends on has_device@ identifier has_device.rel_f, d; @@ void rel_f(struct device *d) { ... * complete(...); ... } === drivers/i2c/i2c-core.c | 10 +--------- include/linux/i2c.h | 1 - 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 39d25a8cb1ad..15cc5902cf89 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -41,7 +41,6 @@ #include #include #include -#include #include #include #include @@ -1184,8 +1183,7 @@ EXPORT_SYMBOL_GPL(i2c_new_dummy); static void i2c_adapter_dev_release(struct device *dev) { - struct i2c_adapter *adap = to_i2c_adapter(dev); - complete(&adap->dev_released); + /* empty, but the driver core insists we need a release function */ } /* @@ -1795,14 +1793,8 @@ void i2c_del_adapter(struct i2c_adapter *adap) /* device name is gone after device_unregister */ dev_dbg(&adap->dev, "adapter [%s] unregistered\n", adap->name); - - /* clean up the sysfs representation */ - init_completion(&adap->dev_released); device_unregister(&adap->dev); - /* wait for sysfs to drop all references */ - wait_for_completion(&adap->dev_released); - /* free bus id */ mutex_lock(&core_lock); idr_remove(&i2c_adapter_idr, adap->nr); diff --git a/include/linux/i2c.h b/include/linux/i2c.h index 524b45ea85a2..1df0c20ce648 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -466,7 +466,6 @@ struct i2c_adapter { int nr; char name[48]; - struct completion dev_released; struct mutex userspace_clients_lock; struct list_head userspace_clients;