From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [02/19] usbnet: smsc95xx: Stop propagation of in_pm From: Oliver Neukum Message-Id: <1546858970.3037.21.camel@suse.com> Date: Mon, 07 Jan 2019 12:02:50 +0100 To: Marek Vasut , netdev@vger.kernel.org Cc: "David S . Miller" , Florian Fainelli , Andrew Lunn , Nisar Sayed , Woojung Huh , linux-usb@vger.kernel.org List-ID: T24gRG8sIDIwMTktMDEtMDMgYXQgMDI6MTAgKzAxMDAsIE1hcmVrIFZhc3V0IHdyb3RlOgo+IFRo ZSBpbmZvcm1hdGlvbiB3aGV0aGVyIHRoZSBTTVNDOTV4eCBpcyBpbl9wbSBvciBub3QgY2FuIGJl IGRlcml2ZWQgZnJvbQo+IHRoZSB1c2JkZXYtPnN1c3BlbmRfY291bnQuIEZpcnN0IHRoaW5nIGNh bGxlZCBpbiBzbXNjOTV4eF9zdXNwZW5kKCkgaXMKPiB1c2JuZXRfc3VzcGVuZCgpLCB3aGljaCBp bmNyZW1lbnRzIHRoZSB1c2JkZXYtPnN1c3BlbmRfY291bnQgYW5kIHNpbmNlCj4gdGhlbiB0aGUg ZHJpdmVyIG9ubHkgY2FsbHMgX25vcG0oKSBmdW5jdGlvbnMgYW5kIGZ1bmN0aW9ucyB3aXRoIGlu X3BtCj4gc2V0IHRvIDEuIFRoZSBzbXNjOTV4eF9yZXN1bWUoKSBkb2VzIHRoZSBpbnZlcnNlLCBp dCBjYWxscyBmdW5jdGlvbnMKPiB3aXRoIF9ub3BtKCkgc3VmZml4IGFuZCB3aXRoIGluX3BtIHNl dCB0byAxIHVudGlsIHVzYm5ldF9yZXN1bWUoKSwgd2hpY2gKPiBzZXRzIHRoZSB1c2JkZXYtPnN1 c3BlbmRfY291bnQgdG8gMC4KCkhpLAoKdW5mb3J0dW5hdGVseSBJIGFtIGZvcmNlZCB0byBkaXNh Z3JlZSBpbiB0aGUgc3Ryb25nZXN0IHRlcm1zLgoKVGhlIGxvZ2ljIGJlaGluZCB0aGlzIHBhdGNo IGlzIHdyb25nLiBUaGUgImluX3BtIiBwYXJhbWV0ZXIKZXhpc3RzIGJlY2F1c2Ugc29tZSBmdW5j dGlvbiBuZWVkIHRvIGJlIGNhbGxlZCBpbiB0aGUgY29kZSBwYXRocwpuZWVkZWQgdG8gaW1wbGVt ZW50IHBvd2VyIG1hbmFnZW1lbnQuIFVuZGVyIHRob3NlIGNpcmN1bXN0YW5jZXMKdGhleSBtdXN0 IG5vdCB0YWtlIGEgcG0gcmVmZXJlbmNlIHRvIGtlZXAgdGhlIGRldmljZSBhd2FrZSwgbGVzdAp0 aGUgZHJpdmVyIGRlYWRsb2NrLgooRm9yIGV4YW1wbGUgX19zbXNjOTV4eF9yZWFkX3JlZykKCkhv d2V2ZXIgZnJvbSBvdGhlciBjb2RlIHBhdGhzIHByZWNpc2VseSB0aGF0IHJlZmVyZW5jZSBtdXN0 IGJlIHRha2VuCmluIG9yZGVyIHRvIG1ha2Ugc3VyZSB0aGF0IHRoZSBkcml2ZXIgZG8gbm90IHRy eSB0byBjb21tdW5pY2F0ZSB3aXRoCmEgc3VzcGVuZGVkIGRldmljZS4KSWYgeW91IGNoZWNrIHRo ZSBzdXNwZW5kIGNvdW50ZXIsIHlvdSB3aWxsIG9taXQgdGhlIG5lY2Vzc2FyeSBnZXR0aW5nCm9m IGEgcmVmZXJlbmNlIHByZWNpc2VseSB3aGVuIGl0IGlzIG5lZWRlZC4gVGhlIGRyaXZlciB3aWxs IG5ldmVyCmRlZGFsb2NrLCBidXQgeW91IHdpbGwgY2F1c2UgbnVtZXJvdXMgcmFjZXMuCgpJIG11 c3Qgc3VnZ2VzdCB0byBzaW1wbHkgZHJvcCB0aGlzIHBhcnQgYW5kIHJlZG8gdGhlIHNlcmllcy4K CglSZWdhcmRzCgkJT2xpdmVyCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oliver Neukum Subject: Re: [PATCH 02/19] usbnet: smsc95xx: Stop propagation of in_pm Date: Mon, 07 Jan 2019 12:02:50 +0100 Message-ID: <1546858970.3037.21.camel@suse.com> References: <20190103011040.25974-1-marex@denx.de> <20190103011040.25974-3-marex@denx.de> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: "David S . Miller" , Florian Fainelli , Andrew Lunn , Nisar Sayed , Woojung Huh , linux-usb@vger.kernel.org To: Marek Vasut , netdev@vger.kernel.org Return-path: Received: from mx2.suse.de ([195.135.220.15]:42690 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726853AbfAGLCz (ORCPT ); Mon, 7 Jan 2019 06:02:55 -0500 In-Reply-To: <20190103011040.25974-3-marex@denx.de> Sender: netdev-owner@vger.kernel.org List-ID: On Do, 2019-01-03 at 02:10 +0100, Marek Vasut wrote: > The information whether the SMSC95xx is in_pm or not can be derived from > the usbdev->suspend_count. First thing called in smsc95xx_suspend() is > usbnet_suspend(), which increments the usbdev->suspend_count and since > then the driver only calls _nopm() functions and functions with in_pm > set to 1. The smsc95xx_resume() does the inverse, it calls functions > with _nopm() suffix and with in_pm set to 1 until usbnet_resume(), which > sets the usbdev->suspend_count to 0. Hi, unfortunately I am forced to disagree in the strongest terms. The logic behind this patch is wrong. The "in_pm" parameter exists because some function need to be called in the code paths needed to implement power management. Under those circumstances they must not take a pm reference to keep the device awake, lest the driver deadlock. (For example __smsc95xx_read_reg) However from other code paths precisely that reference must be taken in order to make sure that the driver do not try to communicate with a suspended device. If you check the suspend counter, you will omit the necessary getting of a reference precisely when it is needed. The driver will never dedalock, but you will cause numerous races. I must suggest to simply drop this part and redo the series. Regards Oliver