From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753214AbcGFIid (ORCPT ); Wed, 6 Jul 2016 04:38:33 -0400 Received: from mail-db5eur01on0133.outbound.protection.outlook.com ([104.47.2.133]:50720 "EHLO EUR01-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751007AbcGFIi3 (ORCPT ); Wed, 6 Jul 2016 04:38:29 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=peda@axentia.se; Subject: Re: [PATCH 2/2] i2c-dev: Don't block the adapter from unregistering To: Viresh Kumar , Wolfram Sang , Jean Delvare References: <021486be2f5425ce2379219a7ac163ee14ba2aba.1467772840.git.viresh.kumar@linaro.org> CC: , , , Johan Hovold , Alex Elder From: Peter Rosin Organization: Axentia Technologies AB Message-ID: <415dcaa7-8ad9-492e-5c5c-605173bc4345@axentia.se> Date: Wed, 6 Jul 2016 10:22:23 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <021486be2f5425ce2379219a7ac163ee14ba2aba.1467772840.git.viresh.kumar@linaro.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [217.210.101.82] X-ClientProxiedBy: DB4PR02CA0021.eurprd02.prod.outlook.com (10.242.174.149) To AM4PR02MB1298.eurprd02.prod.outlook.com (10.165.241.148) X-MS-Office365-Filtering-Correlation-Id: 2dcf0030-8fa1-4a1c-f9ad-08d3a576aaee X-Microsoft-Exchange-Diagnostics: 1;AM4PR02MB1298;2:AXAhnyT4bnyVOuWg1WyuBrQjUnSwrbVeAy1F+jmxPOclWIIHfTEUyelu8aBO0P1acmdxk1BD/A4Lh7omEUc+WDvoBMRY1yrGxKlwEzeDp65pN7Y/rW6c6pmYn6Wyx9vl2PaKg2WyvSPl85FIob2JdFwTW0lAGIVQ3MD+Edo72+Tts7lvDCi7wmOOVDWYW4Bf;3:h/RWHiy5+JssMa0Fgwjz8OIcHbsclB4PbakmFBhCjLWOkNG14deMXgoxi1KMBynQYPnqVqcwIPnbUSA2wrrTASyRKYvS9BmYijiyZFfQXUPQZ9HtawbeUwgNWWkiMcBq;25:dP5WPy/aqaLtMcvd2XHp3RegTZexsNEuwbk4fAf2lBudgD+34g7DqCd2l1phTByaLQ0aPxICrnonm2HyJoi0Rn5JB6cwMoLSSXt7IqhgDRkpciOnf+tlQB/oKwL10ya+l8Yr0t5OsUPxn0pOIeo9A1jB7wmsbh9d6Dmu89LvNasCHfBMyyqv6eha2r8bab8iHp3zM1vjm+k3X0RubVSXM8gYQWD5+eGdZw3zVHySpAihQOnv0JuHSwTE6rppPDRL9ffb5uYAdxbNaXbl62NQoZh/zUrQCaKYCwoq2cpCtU4ugxrV5jjHYd4CezdQAja3OwtiAHLYITt1M5qhC6e5DnjrU8aLv5cdJERH4qC5yXfPL2EuWjyoAu8qvzqd3VDXM//XUT3QBV0KVJSLxOKN+a0eMCIhp3N+OOpElXvgRn8=;31:Yaxntq+RaB5RQdECSPWNqWgOpJEZa6/bmt2jaCbBtGBOQJklOhB3XtepcpGX8eClNpT7d5kUOJVhmfOs+eT43XINAg5WVlmLgiE0OTrhidcV5HNsWu6TvrzDZRlbaHW0Iqu8yZo0xWzwgv6nDpqqeFCtgkPXOLrycww5etA/vafViUf9o0Sxi2advQZU9GrfV3Us0rP5nKYh/JRLw90PgA== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM4PR02MB1298; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040130)(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001)(6041072)(6043046);SRVR:AM4PR02MB1298;BCL:0;PCL:0;RULEID:;SRVR:AM4PR02MB1298; X-Microsoft-Exchange-Diagnostics: 1;AM4PR02MB1298;4:5G+Z/tFl1miVudQPt00w1thMecLqR5D4XcNFHUCwNBeluiDRFDBGmBxs3XlaCH0evvOX1IYyxRW91iJZHxCY3RjeUlPog0qidGw3n9S4pP5ZmzXG7X7XOeLD6oxJa+lebEA2FynUwI1scpkEJenT0uTMgzoOEfQEF1/wSeFSXBdUbOZNa5t75PCZgGH9j7mkIO9JvixBzw3oxlFELePDTS/610PfRDNJ+tszDPZ1gPMtOxkq0wSNSER0qeR4czWBE2omgel4JdNf+87dHOoAu3bZlpE5IqMAku4u+xcKTTfsQrPa/QFRu7BjHg2OYTrEVV0yO6BOyIMqJ2KETLRC67FnbGJAitje7oxIlJ7jk0J3IAigtCfXcpdaJuLVMZQYBo7noJUdHh/anSl8cm3MSw7itnfT8kyfMGhUZMzmCzs= X-Forefront-PRVS: 0995196AA2 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(7916002)(377424004)(199003)(24454002)(189002)(47776003)(64126003)(50466002)(586003)(66066001)(31686004)(42186005)(65956001)(189998001)(2906002)(7736002)(65806001)(305945005)(7846002)(76176999)(117156001)(105586002)(19580395003)(5890100001)(4326007)(50986999)(74482002)(19580405001)(4001350100001)(2950100001)(77096005)(8676002)(54356999)(5001770100001)(68736007)(31696002)(23746002)(6116002)(101416001)(33646002)(3846002)(81156014)(86362001)(36756003)(83506001)(97736004)(92566002)(81166006)(106356001)(230700001)(65826006)(42262002);DIR:OUT;SFP:1102;SCL:1;SRVR:AM4PR02MB1298;H:[192.168.0.125];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;AM4PR02MB1298;23:IZz7Apkly1orvceRC2Jd8X1DuSKO5ZItHRkAy?= =?Windows-1252?Q?vRlxd2ouGJT0xUlDtTZiqzrHJaXZcDK+40I/6azNcoaDSx8Z7jJWYBn8?= =?Windows-1252?Q?0qdfIHsRdbdBYmnzen/n+OOH5tShfvRZfNNUOOYdFBoajvZWtcQMhqbt?= =?Windows-1252?Q?Cyqi3zfzsRs8dto8Q9KHuglsvWnpnhAlvclk6GFRDctp86mUUALPPtcH?= =?Windows-1252?Q?kpkocLCzzijUMQzKohF58qJXgzIrqjfgyiUf5yM7DXWUar3qMFhPTa+a?= =?Windows-1252?Q?Rd5tmMdXLQvIkrsF0gH3ik1TiSP1NXNMtu3ujHPWyRaI7rXFDLLm+wzO?= =?Windows-1252?Q?3ycuuV/UZ/mLrf2OdiYYwQdufkB4Q7VueF7iFLj+xEs1iUIYk2bm/dSC?= =?Windows-1252?Q?q15Ms3pPsXVFB03+MTTY2D+iIxT44sDpJS+6LAi6wnnITMTmoqa/IzX8?= =?Windows-1252?Q?25sirtXD6yhgGGEqx1WeDIcnDxO71ri4z+6m1dGPvjZKFg8QTaCFjbV/?= =?Windows-1252?Q?27WxYQR0aKsXUqwhnOkNbpOKkkx0Go9qM3dnnAKHW57BiHuIqTlR5w9D?= =?Windows-1252?Q?1yVOGsiVggYifYzpsTAB2dq0WfBeHltiY4mS62hbmgcHSosbc4zt7UBj?= =?Windows-1252?Q?Xsr6TGjJiRP5aBmmS9KrBn/0WlGX8Q6rYGwN/U9ApvlglV4qre42HEqH?= =?Windows-1252?Q?Ol0El3ZxE62WQGPeJrZstgXM/MGyZD+EeDzHCF+j6pPK7X2TlYFlXmQU?= =?Windows-1252?Q?ZllTHQzlipbgtVrfOlV8X5zuIX3jV7RlGIgllPDGdh50JnSf3y9OKFtl?= =?Windows-1252?Q?HJwnR8F5F3+XPd3IlEfSoiDqpphMMg5a5YSWQK1+xbMSHYb4S5lAlX9X?= =?Windows-1252?Q?d+wZDTI5DctTRZXlmugNqOjjCiDkN7GqTdSS0uwu0hukRwN7IsKJ1MIi?= =?Windows-1252?Q?xv03vJh08zhNhkRzDEE3YwMSWkUtLbUivGScLdapn9LTBJQvy28zSO4p?= =?Windows-1252?Q?jHdIGfn70IqPIk/pVTaaSX2RzUkefmRax8YUAhoHY9CiS7cH0lVEHNel?= =?Windows-1252?Q?SFK5xn3f+wREfnHhokS49aQTfDmrr5M6hiYr2adcPr8F3LCRGPBTN+Yc?= =?Windows-1252?Q?eW69iYYTDOTjPRvlWnm08lFToK0GegzpwfjMbcsi2pUKaBiEi3cKOFq2?= =?Windows-1252?Q?HOtunkhq46AOhw940fKb7rz/hsHLSKmeSzmKsfsAOGhrMD+nE57S3tTm?= =?Windows-1252?Q?DxhomJRQTSjrTDCPqMNG2BI+5R3P5cVoEeUqKMcM3tmlNLxX3HuuVwe+?= =?Windows-1252?Q?oCnjaGo/Smyn7M0tz/Ozxp7Fuga69OF6Hjpk60fb2ENmc6/Gux7WedjI?= =?Windows-1252?Q?Ef5xxQTlz7CWGD13q6H37Qc5OtnzZb0ZDZaGPivZtYoVw42ol8DVjEni?= =?Windows-1252?Q?7iiPypYOmNUGhvFYgOqJBgeMklXLcTuKQVCEoWVtQ=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;AM4PR02MB1298;6:d/ECxytC3HHFqNZtN10xbwMkm3C6LUOFL/DI1RNVZ1fcJes5o0dlra4R4uTBV1WETUj12Ffv1V7t18VCJZ4NlAqatoIbTamY5nicLTE1D3Dgfi8Vv/ZCXqvFmbZGhVUz90RLt1/lD7VT/ISiSvFb7C0nvNRP+SiwBwfYC1UzYAboDxmTllFCHtVQjrMsZr5lJp0xXHzjNS6ULL6RY4QSSQM4eYM0BMK4gF+BypoWsgqIHA/F0sS1bkmZVjc5DDKQZbXGrramh06xONvW76VyeTEyTom0hkefrcCUf7ke7UI/9RRaNEjaWCPB9r8P854s;5:e00uqIgDtfMzBtLkj4uy8sfih4krYzCDJjNDqqFWZbQBFLQCAJTxCb6ydXF4NHUrFcUFDQ5r+1lv2tO38rCvECS7wUzQPbX1WOxZdOyBDfSaKtFZmyyTX7Nxult5qQ58s47dA4ne9r4XfPUJph04/Q==;24:xzGGsCz+zm3DkzUchw4Q5bGTA9ahH0nqO6EjjfXp3i7vRvy0XTbAEXSLyQJq11usGGyGdlORBtdHE1ivEUcJfAbRCtPCd7IE6PuETPn1k2k=;7:aDxipXQVfaTL3AWqQV8oqimZFm814sC28qvBKVtBFgJ39Td75Cy5N/SPPBz8Tk9e6nOq//BACqI2UnG5lY9ZJBIgXbz0EnkoMAEVgLWLyBfBwToG+pZq/Wwf915aF39Hw0uFwMkqNMYaPddO3CqftFJuM8PbS2Or27zlfT7t1TaTDtIeG7SqcqcCYtJaSWDJ+iQJ/gzT60j/I3Syag8H6lgPcmTIh1ib8iC0rBZmqUwaSTO77Izjfd0KQfTiyN6n SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: axentia.se X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jul 2016 08:22:27.0965 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR02MB1298 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016-07-06 04:57, Viresh Kumar wrote: > The i2c-dev calls i2c_get_adapter() from the .open() callback, which > doesn't let the adapter device unregister unless the .close() callback > is called. > > On some platforms (like Google ARA), this doesn't let the modules > (hardware attached to the phone) eject from the phone as the cleanup > path for the module hasn't finished yet (i2c adapter not removed). > > We can't let the userspace block the kernel forever in such cases. > > Fix this by calling i2c_get_adapter() from all other file operations, > i.e. read/write/ioctl, to make sure the adapter doesn't get away while > we are in the middle of a operation, but not otherwise. In .open() we > will release the adapter device before returning and so if there is no > data transfer in progress, then the i2c-dev doesn't block the adapter > from unregistering. > > Signed-off-by: Viresh Kumar > --- > drivers/i2c/i2c-dev.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++----- > include/linux/i2c.h | 1 + > 2 files changed, 66 insertions(+), 7 deletions(-) > > diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c > index 66f323fd3982..b2562603daa9 100644 > --- a/drivers/i2c/i2c-dev.c > +++ b/drivers/i2c/i2c-dev.c > @@ -142,13 +142,25 @@ static ssize_t i2cdev_read(struct file *file, char __user *buf, size_t count, > int ret; > > struct i2c_client *client = file->private_data; > + struct i2c_adapter *adap; > + > + adap = i2c_get_adapter(client->adapter_nr); > + if (!adap) > + return -ENODEV; > + > + if (adap != client->adapter) { > + ret = -EINVAL; > + goto put_adapter; > + } I don't see how this can work with the i2c-demux-pinctrl driver. I also wonder if/how other muxes handle this relaxed adapter lifetime thingy? Out of curiosity, why would client->adapter change anyway? (that is, if not because of a demux-pinctrl op) Or maybe I'm just paranoid, but this is not obvious... Cheers, Peter From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Rosin Subject: Re: [PATCH 2/2] i2c-dev: Don't block the adapter from unregistering Date: Wed, 6 Jul 2016 10:22:23 +0200 Message-ID: <415dcaa7-8ad9-492e-5c5c-605173bc4345@axentia.se> References: <021486be2f5425ce2379219a7ac163ee14ba2aba.1467772840.git.viresh.kumar@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-db5eur01on0133.outbound.protection.outlook.com ([104.47.2.133]:50720 "EHLO EUR01-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751007AbcGFIi3 (ORCPT ); Wed, 6 Jul 2016 04:38:29 -0400 In-Reply-To: <021486be2f5425ce2379219a7ac163ee14ba2aba.1467772840.git.viresh.kumar@linaro.org> Sender: linux-i2c-owner@vger.kernel.org List-Id: linux-i2c@vger.kernel.org To: Viresh Kumar , Wolfram Sang , Jean Delvare Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org, Johan Hovold , Alex Elder On 2016-07-06 04:57, Viresh Kumar wrote: > The i2c-dev calls i2c_get_adapter() from the .open() callback, which > doesn't let the adapter device unregister unless the .close() callback > is called. > > On some platforms (like Google ARA), this doesn't let the modules > (hardware attached to the phone) eject from the phone as the cleanup > path for the module hasn't finished yet (i2c adapter not removed). > > We can't let the userspace block the kernel forever in such cases. > > Fix this by calling i2c_get_adapter() from all other file operations, > i.e. read/write/ioctl, to make sure the adapter doesn't get away while > we are in the middle of a operation, but not otherwise. In .open() we > will release the adapter device before returning and so if there is no > data transfer in progress, then the i2c-dev doesn't block the adapter > from unregistering. > > Signed-off-by: Viresh Kumar > --- > drivers/i2c/i2c-dev.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++----- > include/linux/i2c.h | 1 + > 2 files changed, 66 insertions(+), 7 deletions(-) > > diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c > index 66f323fd3982..b2562603daa9 100644 > --- a/drivers/i2c/i2c-dev.c > +++ b/drivers/i2c/i2c-dev.c > @@ -142,13 +142,25 @@ static ssize_t i2cdev_read(struct file *file, char __user *buf, size_t count, > int ret; > > struct i2c_client *client = file->private_data; > + struct i2c_adapter *adap; > + > + adap = i2c_get_adapter(client->adapter_nr); > + if (!adap) > + return -ENODEV; > + > + if (adap != client->adapter) { > + ret = -EINVAL; > + goto put_adapter; > + } I don't see how this can work with the i2c-demux-pinctrl driver. I also wonder if/how other muxes handle this relaxed adapter lifetime thingy? Out of curiosity, why would client->adapter change anyway? (that is, if not because of a demux-pinctrl op) Or maybe I'm just paranoid, but this is not obvious... Cheers, Peter