Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1244529 > unrolled thread

RE: [PATCH 3/3] Thermal: do thermal zone update after a cooling device registered

Started by"Chen, Yu C" <yu.c.chen@intel.com>
First post2015-10-12 11:30 +0200
Last post2015-10-14 21:30 +0200
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [PATCH 3/3] Thermal: do thermal zone update after a cooling  device registered "Chen, Yu C" <yu.c.chen@intel.com> - 2015-10-12 11:30 +0200
    Re: [PATCH 3/3] Thermal: do thermal zone update after a cooling  device registered Javi Merino <javi.merino@arm.com> - 2015-10-14 19:10 +0200
      RE: [PATCH 3/3] Thermal: do thermal zone update after a cooling  device registered "Chen, Yu C" <yu.c.chen@intel.com> - 2015-10-14 21:30 +0200
        Re: [PATCH 3/3] Thermal: do thermal zone update after a cooling  device registered Javi Merino <javi.merino@arm.com> - 2015-10-15 16:10 +0200
      RE: [PATCH 3/3] Thermal: do thermal zone update after a cooling  device registered "Chen, Yu C" <yu.c.chen@intel.com> - 2015-10-14 21:30 +0200

#1244529 — RE: [PATCH 3/3] Thermal: do thermal zone update after a cooling device registered

From"Chen, Yu C" <yu.c.chen@intel.com>
Date2015-10-12 11:30 +0200
SubjectRE: [PATCH 3/3] Thermal: do thermal zone update after a cooling device registered
Message-ID<qiHAE-38y-39@gated-at.bofh.it>
SGksIEphdmkNClNvcnJ5IGZvciBteSBsYXRlIHJlc3BvbnNlLA0KDQo+IC0tLS0tT3JpZ2luYWwg
TWVzc2FnZS0tLS0tDQo+IEZyb206IEphdmkgTWVyaW5vIFttYWlsdG86amF2aS5tZXJpbm9AYXJt
LmNvbV0NCj4gU2VudDogV2VkbmVzZGF5LCBTZXB0ZW1iZXIgMzAsIDIwMTUgMTI6MDIgQU0NCj4g
VG86IENoZW4sIFl1IEMNCj4gQ2M6IGxpbnV4LXBtQHZnZXIua2VybmVsLm9yZzsgZWR1YmV6dmFs
QGdtYWlsLmNvbTsgWmhhbmcsIFJ1aTsgbGludXgtDQo+IGtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7
IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRDSCAzLzNdIFRoZXJt
YWw6IGRvIHRoZXJtYWwgem9uZSB1cGRhdGUgYWZ0ZXIgYSBjb29saW5nDQo+IGRldmljZSByZWdp
c3RlcmVkDQo+IA0KPiBIaSBZdSwNCj4gDQo+IE9uIE1vbiwgU2VwIDI4LCAyMDE1IGF0IDA2OjUy
OjAwUE0gKzAxMDAsIENoZW4sIFl1IEMgd3JvdGU6DQo+ID4gSGksIEphdmksDQo+ID4NCj4gPiA+
IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiBGcm9tOiBKYXZpIE1lcmlubyBbbWFp
bHRvOmphdmkubWVyaW5vQGFybS5jb21dDQo+ID4gPiBTZW50OiBNb25kYXksIFNlcHRlbWJlciAy
OCwgMjAxNSAxMDoyOSBQTQ0KPiA+ID4gVG86IENoZW4sIFl1IEMNCj4gPiA+IENjOiBsaW51eC1w
bUB2Z2VyLmtlcm5lbC5vcmc7IGVkdWJlenZhbEBnbWFpbC5jb207IFpoYW5nLCBSdWk7DQo+ID4g
PiBsaW51eC0ga2VybmVsQHZnZXIua2VybmVsLm9yZzsgc3RhYmxlQHZnZXIua2VybmVsLm9yZw0K
PiA+ID4gU3ViamVjdDogUmU6IFtQQVRDSCAzLzNdIFRoZXJtYWw6IGRvIHRoZXJtYWwgem9uZSB1
cGRhdGUgYWZ0ZXIgYQ0KPiA+ID4gY29vbGluZyBkZXZpY2UgcmVnaXN0ZXJlZA0KPiA+ID4NCj4g
PiA+IE9uIFN1biwgU2VwIDI3LCAyMDE1IGF0IDA2OjQ4OjQ0QU0gKzAxMDAsIENoZW4gWXUgd3Jv
dGU6DQo+ID4gPiA+IEZyb206IFpoYW5nIFJ1aSA8cnVpLnpoYW5nQGludGVsLmNvbT4NCj4gPiA+
ID4NCj4gPiA+ID4NCj4gPiA+DQo+ID4gPiBJIHRoaW5rIHlvdSBuZWVkIHRvIGhvbGQgY2Rldi0+
bG9jayBoZXJlLCB0byBtYWtlIHN1cmUgdGhhdCBubw0KPiA+ID4gdGhlcm1hbCB6b25lIGlzIGFk
ZGVkIG9yIHJlbW92ZWQgZnJvbSBjZGV2LT50aGVybWFsX2luc3RhbmNlcyB3aGlsZQ0KPiB5b3Ug
YXJlIGxvb3BpbmcuDQo+ID4gPg0KPiA+IEFoIHJpZ2h0LCB3aWxsIGFkZC4gSWYgSSBhZGQgdGhl
IGNkZXYgLT5sb2NrIGhlcmUsIHdpbGwgdGhlcmUgYmUgYQ0KPiA+IEFCLUJBIGxvY2sgd2l0aCB0
aGVybWFsX3pvbmVfdW5iaW5kX2Nvb2xpbmdfZGV2aWNlPw0KPiANCj4gWW91J3JlIHJpZ2h0LCBp
dCBjb3VsZCBsZWFkIHRvIGEgZGVhZGxvY2suICBUaGUgbG9ja3MgY2FuJ3QgYmUgc3dhcHBlZCBi
ZWNhdXNlDQo+IHRoYXQgd29uJ3Qgd29yayBpbiBzdGVwX3dpc2UuDQo+IA0KPiBUaGUgYmVzdCB3
YXkgdGhhdCBJIGNhbiB0aGluayBvZiBhY2Nlc3NpbmcgdGhlcm1hbF9pbnN0YW5jZXMgYXRvbWlj
YWxseSBpcyBieQ0KPiBtYWtpbmcgaXQgUkNVIHByb3RlY3RlZCBpbnN0ZWFkIG9mIHdpdGggbXV0
ZXhlcy4NCj4gV2hhdCBkbyB5b3UgdGhpbms/DQo+IA0KUkNVIHdvdWxkIG5lZWQgZXh0cmEgc3Bp
bmxvY2tzIHRvIHByb3RlY3QgdGhlIGxpc3QsIGFuZCBuZWVkIHRvIHN5bmNfcmN1IGFmdGVyIHdl
IGRlbGV0ZQ0Kb25lIGluc3RhbmNlIGZyb20gdGhlcm1hbF9pbnN0YW5jZSBsaXN0LCAgSSB0aGlu
ayBpdCBpcyB0b28gY29tcGxpY2F0ZWQgZm9yIG1lIHRvIHJld3JpdGU6ICgNCkhvdyBhYm91dCB1
c2luZyB0aGVybWFsX2xpc3RfbG9jayBpbnN0ZWFkIG9mIGNkZXYgLT5sb2NrPw0KVGhpcyBndXkg
c2hvdWxkIGJlIGJpZyBlbm91Z2ggdG8gcHJvdGVjdCB0aGUgZGV2aWNlLnRoZXJtYWxfaW5zdGFu
Y2UgbGlzdC4NCg0KPiANCj4gPiA+IFdoeSBsaXN0X2Zvcl9lYWNoX2VudHJ5X3NhZmUoKSA/ICBZ
b3UgYXJlIG5vdCBnb2luZyB0byByZW1vdmUgYW55DQo+ID4gPiBlbnRyeSwgc28geW91IGNhbiBq
dXN0IHVzZSBsaXN0X2Zvcl9lYWNoX2VudHJ5KCkNCj4gPiA+DQo+ID4gPg0KPiA+ID4gV2h5IGlz
IHRoaXMgc28gY29tcGxpY2F0ZWQ/ICBDYW4ndCB5b3UganVzdCBkbzoNCj4gPiA+DQo+ID4gPiAJ
bGlzdF9mb3JfZWFjaF9lbnRyeShwb3MsICZjZGV2LT50aGVybWFsX2luc3RhbmNlcywgY2Rldl9u
b2RlKQ0KPiA+ID4gICAgICAgICAJdGhlcm1hbF96b25lX2RldmljZV91cGRhdGUocG9zLT50eik7
DQo+ID4gPg0KPiA+DQo+ID4gVGhpcyBpcyBhbiBvcHRpbWl6YXRpb24gaGVyZToNCj4gPiBJZ25v
cmUgdGhlcm1hbCBpbnN0YW5jZSB0aGF0IHJlZmVycyB0byB0aGUgc2FtZSB0aGVybWFsIHpvbmUg
aW4gdGhpcw0KPiA+IGxvb3AsIHRoaXMgd29ya3MgYmVjYXVzZSBiaW5kX2NkZXYoKSBhbHdheXMg
YmluZHMgdGhlIGNvb2xpbmcgZGV2aWNlDQo+ID4gdG8gb25lIHRoZXJtYWwgem9uZSBmaXJzdCwg
YW5kIHRoZW4gYmluZHMgdG8gdGhlIG5leHQgdGhlcm1hbCB6b25lLg0KPiANCj4gSXQgaGFzIHRh
a2VuIG1lIGEgd2hpbGUgdG8gdW5kZXJzdGFuZCB0aGlzIG9wdGltaXphdGlvbi4gIFBsZWFzZSBk
b2N1bWVudA0KPiBib3RoICJpZiJzIGluIHRoZSBjb2RlLiAgRm9yIHRoZSBmaXJzdCAiaWYiIG1h
eWJlIHlvdSBjYW4gdXNlDQo+IGxpc3RfaXNfbGFzdCgpIHRvIG1ha2UgaXQgZWFzaWVyIHRvIHVu
ZGVyc3RhbmQgdGhhdCB5b3UncmUgbG9va2luZyBmb3IgdGhlIGxhc3QNCj4gZWxlbWVudCBpbiB0
aGUgbGlzdDoNCj4gDQo+IAkJaWYgKGxpc3RfaXNfbGFzdCgmcG9zLT5jZGV2X25vZGUsICZjZGV2
LQ0KPiA+dGhlcm1hbF9pbnN0YW5jZXMpKSB7DQo+IAkJCXRoZXJtYWxfem9uZV9kZXZpY2VfdXBk
YXRlKHBvcy0+dHopOw0KPiANClN1cmUsIG9rDQo+IEZvciB0aGUgc2Vjb25kICJpZiIgeW91IGNh
biBzYXkgdGhhdCB5b3Ugb25seSBuZWVkIHRvIHJ1bg0KPiB0aGVybWFsX3pvbmVfZGV2aWNlX3Vw
ZGF0ZSgpIG9uY2UgcGVyIHRoZXJtYWwgem9uZSwgZXZlbiB0aG91Z2gNCj4gbXVsdGlwbGUgdGhl
cm1hbCBpbnN0YW5jZXMgbWF5IHJlZmVyIHRvIHRoZSBzYW1lIHRoZXJtYWwgem9uZS4NCj4gDQpP
Sw0KDQoNCkJlc3QgUmVnYXJkcywNCll1DQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1247009

FromJavi Merino <javi.merino@arm.com>
Date2015-10-14 19:10 +0200
Message-ID<qjxIT-4Xf-19@gated-at.bofh.it>
In reply to#1244529
On Mon, Oct 12, 2015 at 09:23:28AM +0000, Chen, Yu C wrote:
> Hi, Javi
> Sorry for my late response,
> 
> > -----Original Message-----
> > From: Javi Merino [mailto:javi.merino@arm.com]
> > Sent: Wednesday, September 30, 2015 12:02 AM
> > To: Chen, Yu C
> > Cc: linux-pm@vger.kernel.org; edubezval@gmail.com; Zhang, Rui; linux-
> > kernel@vger.kernel.org; stable@vger.kernel.org
> > Subject: Re: [PATCH 3/3] Thermal: do thermal zone update after a cooling
> > device registered
> > 
> > Hi Yu,
> > 
> > On Mon, Sep 28, 2015 at 06:52:00PM +0100, Chen, Yu C wrote:
> > > Hi, Javi,
> > >
> > > > -----Original Message-----
> > > > From: Javi Merino [mailto:javi.merino@arm.com]
> > > > Sent: Monday, September 28, 2015 10:29 PM
> > > > To: Chen, Yu C
> > > > Cc: linux-pm@vger.kernel.org; edubezval@gmail.com; Zhang, Rui;
> > > > linux- kernel@vger.kernel.org; stable@vger.kernel.org
> > > > Subject: Re: [PATCH 3/3] Thermal: do thermal zone update after a
> > > > cooling device registered
> > > >
> > > > On Sun, Sep 27, 2015 at 06:48:44AM +0100, Chen Yu wrote:
> > > > > From: Zhang Rui <rui.zhang@intel.com>
> > > > >
> > > > >
> > > >
> > > > I think you need to hold cdev->lock here, to make sure that no
> > > > thermal zone is added or removed from cdev->thermal_instances while
> > you are looping.
> > > >
> > > Ah right, will add. If I add the cdev ->lock here, will there be a
> > > AB-BA lock with thermal_zone_unbind_cooling_device?
> > 
> > You're right, it could lead to a deadlock.  The locks can't be swapped because
> > that won't work in step_wise.
> > 
> > The best way that I can think of accessing thermal_instances atomically is by
> > making it RCU protected instead of with mutexes.
> > What do you think?
> > 
> RCU would need extra spinlocks to protect the list, and need to sync_rcu after we delete
> one instance from thermal_instance list,  I think it is too complicated for me to rewrite: (
> How about using thermal_list_lock instead of cdev ->lock?
> This guy should be big enough to protect the device.thermal_instance list.

thermal_list_lock protects thermal_tz_list and thermal_cdev_list, but
it doesn't protect the thermal_instances list.  For example,
thermal_zone_bind_cooling_device() adds a cooling device to the
cdev->thermal_instances list without taking thermal_tz_list.

To sum up, you have to protect accessing the cdev->thermal_instances
list but with the current locking scheme, you would create an AB-BA
deadlock.  As I see it you would have to change the locking scheme to
either RCU or add a new mutex that protects the
cdev->thermal_instances and tz->thermal_instances lists and change all
accesses to them to make sure they comply with the new locking scheme.

Is there a better way of solving this?  Cheers,
Javi


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247103

From"Chen, Yu C" <yu.c.chen@intel.com>
Date2015-10-14 21:30 +0200
Message-ID<qjzUm-83J-5@gated-at.bofh.it>
In reply to#1247009
SGksSmF2aQ0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IEphdmkgTWVy
aW5vIFttYWlsdG86amF2aS5tZXJpbm9AYXJtLmNvbV0NCj4gU2VudDogVGh1cnNkYXksIE9jdG9i
ZXIgMTUsIDIwMTUgMTowOCBBTQ0KPiBUbzogQ2hlbiwgWXUgQw0KPiBDYzogbGludXgtcG1Admdl
ci5rZXJuZWwub3JnOyBlZHViZXp2YWxAZ21haWwuY29tOyBaaGFuZywgUnVpOyBsaW51eC0NCj4g
a2VybmVsQHZnZXIua2VybmVsLm9yZzsgc3RhYmxlQHZnZXIua2VybmVsLm9yZzsgUGFuZHJ1dmFk
YSwgU3Jpbml2YXMNCj4gU3ViamVjdDogUmU6IFtQQVRDSCAzLzNdIFRoZXJtYWw6IGRvIHRoZXJt
YWwgem9uZSB1cGRhdGUgYWZ0ZXIgYSBjb29saW5nDQo+IGRldmljZSByZWdpc3RlcmVkDQo+IA0K
PiBPbiBNb24sIE9jdCAxMiwgMjAxNSBhdCAwOToyMzoyOEFNICswMDAwLCBDaGVuLCBZdSBDIHdy
b3RlOg0KPiA+IEhpLCBKYXZpDQo+ID4gU29ycnkgZm9yIG15IGxhdGUgcmVzcG9uc2UsDQo+ID4N
Cj4gPiA+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiBGcm9tOiBKYXZpIE1lcmlu
byBbbWFpbHRvOmphdmkubWVyaW5vQGFybS5jb21dDQo+ID4gPiBTZW50OiBXZWRuZXNkYXksIFNl
cHRlbWJlciAzMCwgMjAxNSAxMjowMiBBTQ0KPiA+ID4gVG86IENoZW4sIFl1IEMNCj4gPiA+IENj
OiBsaW51eC1wbUB2Z2VyLmtlcm5lbC5vcmc7IGVkdWJlenZhbEBnbWFpbC5jb207IFpoYW5nLCBS
dWk7DQo+ID4gPiBsaW51eC0ga2VybmVsQHZnZXIua2VybmVsLm9yZzsgc3RhYmxlQHZnZXIua2Vy
bmVsLm9yZw0KPiA+ID4gU3ViamVjdDogUmU6IFtQQVRDSCAzLzNdIFRoZXJtYWw6IGRvIHRoZXJt
YWwgem9uZSB1cGRhdGUgYWZ0ZXIgYQ0KPiA+ID4gY29vbGluZyBkZXZpY2UgcmVnaXN0ZXJlZA0K
PiA+ID4NCj4gPiA+IEhpIFl1LA0KPiA+ID4NCj4gPiA+IE9uIE1vbiwgU2VwIDI4LCAyMDE1IGF0
IDA2OjUyOjAwUE0gKzAxMDAsIENoZW4sIFl1IEMgd3JvdGU6DQo+ID4gPiA+IEhpLCBKYXZpLA0K
PiA+ID4gPg0KPiA+ID4gPiA+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiA+ID4g
RnJvbTogSmF2aSBNZXJpbm8gW21haWx0bzpqYXZpLm1lcmlub0Bhcm0uY29tXQ0KPiA+ID4gPiA+
IFNlbnQ6IE1vbmRheSwgU2VwdGVtYmVyIDI4LCAyMDE1IDEwOjI5IFBNDQo+ID4gPiA+ID4gVG86
IENoZW4sIFl1IEMNCj4gPiA+ID4gPiBDYzogbGludXgtcG1Admdlci5rZXJuZWwub3JnOyBlZHVi
ZXp2YWxAZ21haWwuY29tOyBaaGFuZywgUnVpOw0KPiA+ID4gPiA+IGxpbnV4LSBrZXJuZWxAdmdl
ci5rZXJuZWwub3JnOyBzdGFibGVAdmdlci5rZXJuZWwub3JnDQo+ID4gPiA+ID4gU3ViamVjdDog
UmU6IFtQQVRDSCAzLzNdIFRoZXJtYWw6IGRvIHRoZXJtYWwgem9uZSB1cGRhdGUgYWZ0ZXIgYQ0K
PiA+ID4gPiA+IGNvb2xpbmcgZGV2aWNlIHJlZ2lzdGVyZWQNCj4gPiA+ID4gPg0KPiA+ID4gPiA+
IE9uIFN1biwgU2VwIDI3LCAyMDE1IGF0IDA2OjQ4OjQ0QU0gKzAxMDAsIENoZW4gWXUgd3JvdGU6
DQo+ID4gPiA+ID4gPiBGcm9tOiBaaGFuZyBSdWkgPHJ1aS56aGFuZ0BpbnRlbC5jb20+DQo+ID4g
PiA+ID4gPg0KPiA+ID4gPiA+ID4NCj4gPiA+ID4gPg0KPiA+ID4gPiA+IEkgdGhpbmsgeW91IG5l
ZWQgdG8gaG9sZCBjZGV2LT5sb2NrIGhlcmUsIHRvIG1ha2Ugc3VyZSB0aGF0IG5vDQo+ID4gPiA+
ID4gdGhlcm1hbCB6b25lIGlzIGFkZGVkIG9yIHJlbW92ZWQgZnJvbSBjZGV2LT50aGVybWFsX2lu
c3RhbmNlcw0KPiA+ID4gPiA+IHdoaWxlDQo+ID4gPiB5b3UgYXJlIGxvb3BpbmcuDQo+ID4gPiA+
ID4NCj4gPiA+ID4gQWggcmlnaHQsIHdpbGwgYWRkLiBJZiBJIGFkZCB0aGUgY2RldiAtPmxvY2sg
aGVyZSwgd2lsbCB0aGVyZSBiZSBhDQo+ID4gPiA+IEFCLUJBIGxvY2sgd2l0aCB0aGVybWFsX3pv
bmVfdW5iaW5kX2Nvb2xpbmdfZGV2aWNlPw0KPiA+ID4NCj4gPiA+IFlvdSdyZSByaWdodCwgaXQg
Y291bGQgbGVhZCB0byBhIGRlYWRsb2NrLiAgVGhlIGxvY2tzIGNhbid0IGJlDQo+ID4gPiBzd2Fw
cGVkIGJlY2F1c2UgdGhhdCB3b24ndCB3b3JrIGluIHN0ZXBfd2lzZS4NCj4gPiA+DQo+ID4gPiBU
aGUgYmVzdCB3YXkgdGhhdCBJIGNhbiB0aGluayBvZiBhY2Nlc3NpbmcgdGhlcm1hbF9pbnN0YW5j
ZXMNCj4gPiA+IGF0b21pY2FsbHkgaXMgYnkgbWFraW5nIGl0IFJDVSBwcm90ZWN0ZWQgaW5zdGVh
ZCBvZiB3aXRoIG11dGV4ZXMuDQo+ID4gPiBXaGF0IGRvIHlvdSB0aGluaz8NCj4gPiA+DQo+ID4g
UkNVIHdvdWxkIG5lZWQgZXh0cmEgc3BpbmxvY2tzIHRvIHByb3RlY3QgdGhlIGxpc3QsIGFuZCBu
ZWVkIHRvDQo+ID4gc3luY19yY3UgYWZ0ZXIgd2UgZGVsZXRlIG9uZSBpbnN0YW5jZSBmcm9tIHRo
ZXJtYWxfaW5zdGFuY2UgbGlzdCwgIEkNCj4gPiB0aGluayBpdCBpcyB0b28gY29tcGxpY2F0ZWQg
Zm9yIG1lIHRvIHJld3JpdGU6ICggSG93IGFib3V0IHVzaW5nDQo+IHRoZXJtYWxfbGlzdF9sb2Nr
IGluc3RlYWQgb2YgY2RldiAtPmxvY2s/DQo+ID4gVGhpcyBndXkgc2hvdWxkIGJlIGJpZyBlbm91
Z2ggdG8gcHJvdGVjdCB0aGUgZGV2aWNlLnRoZXJtYWxfaW5zdGFuY2UgbGlzdC4NCj4gDQo+IHRo
ZXJtYWxfbGlzdF9sb2NrIHByb3RlY3RzIHRoZXJtYWxfdHpfbGlzdCBhbmQgdGhlcm1hbF9jZGV2
X2xpc3QsIGJ1dCBpdA0KPiBkb2Vzbid0IHByb3RlY3QgdGhlIHRoZXJtYWxfaW5zdGFuY2VzIGxp
c3QuICBGb3IgZXhhbXBsZSwNCj4gdGhlcm1hbF96b25lX2JpbmRfY29vbGluZ19kZXZpY2UoKSBh
ZGRzIGEgY29vbGluZyBkZXZpY2UgdG8gdGhlDQo+IGNkZXYtPnRoZXJtYWxfaW5zdGFuY2VzIGxp
c3Qgd2l0aG91dCB0YWtpbmcgdGhlcm1hbF90el9saXN0Lg0KPiANCkJlZm9yZSB0aGVybWFsX3pv
bmVfYmluZF9jb29saW5nX2RldmljZSBpcyBpbnZva2VkLA0KdGhlIHRoZXJtYWxfbGlzdF9sb2Nr
IHdpbGwgYmUgZmlyc3RseSBncmlwcGVkOg0KDQpzdGF0aWMgdm9pZCBiaW5kX2NkZXYoc3RydWN0
IHRoZXJtYWxfY29vbGluZ19kZXZpY2UgKmNkZXYpDQp7DQptdXRleF9sb2NrKCZ0aGVybWFsX2xp
c3RfbG9jayk7DQplaXRoZXIgdHotPm9wcy0+YmluZCAgICA6ICAgdGhlcm1hbF96b25lX2JpbmRf
Y29vbGluZ19kZXZpY2UNCm9yIF9fYmluZCgpICA6ICAgdGhlcm1hbF96b25lX2JpbmRfY29vbGlu
Z19kZXZpY2UNCm11dGV4X3VubG9jaygmdGhlcm1hbF9saXN0X2xvY2spOw0KfQ0KDQpBbmQgaXQg
aXMgdGhlIHNhbWUgYXMgaW4gIHBhc3NpdmVfc3RvcmUuDQpTbyB3aGVuIGNvZGUgaXMgdHJ5aW5n
IHRvIGFkZC9kZWxldGUgdGhlcm1hbF9pbnN0YW5jZSBvZiBjZGV2LA0KaGUgaGFzIGFscmVhZHkg
aG9sZCB0aGVybWFsX2xpc3RfbG9jayBJTU8uIE9yIGRvIEkgbWlzcyBhbnl0aGluZz8NCg0KQmVz
dCBSZWdhcmRzLA0KWXUNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247813

FromJavi Merino <javi.merino@arm.com>
Date2015-10-15 16:10 +0200
Message-ID<qjRoe-dR-1@gated-at.bofh.it>
In reply to#1247103
On Wed, Oct 14, 2015 at 07:23:55PM +0000, Chen, Yu C wrote:
> > -----Original Message-----
> > From: Javi Merino [mailto:javi.merino@arm.com]
> > Sent: Thursday, October 15, 2015 1:08 AM
> > To: Chen, Yu C
> > Cc: linux-pm@vger.kernel.org; edubezval@gmail.com; Zhang, Rui; linux-
> > kernel@vger.kernel.org; stable@vger.kernel.org; Pandruvada, Srinivas
> > Subject: Re: [PATCH 3/3] Thermal: do thermal zone update after a cooling
> > device registered
> > 
> > On Mon, Oct 12, 2015 at 09:23:28AM +0000, Chen, Yu C wrote:
> > > Hi, Javi
> > > Sorry for my late response,
> > >
> > > > -----Original Message-----
> > > > From: Javi Merino [mailto:javi.merino@arm.com]
> > > > Sent: Wednesday, September 30, 2015 12:02 AM
> > > > To: Chen, Yu C
> > > > Cc: linux-pm@vger.kernel.org; edubezval@gmail.com; Zhang, Rui;
> > > > linux- kernel@vger.kernel.org; stable@vger.kernel.org
> > > > Subject: Re: [PATCH 3/3] Thermal: do thermal zone update after a
> > > > cooling device registered
> > > >
> > > > Hi Yu,
> > > >
> > > > On Mon, Sep 28, 2015 at 06:52:00PM +0100, Chen, Yu C wrote:
> > > > > Hi, Javi,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Javi Merino [mailto:javi.merino@arm.com]
> > > > > > Sent: Monday, September 28, 2015 10:29 PM
> > > > > > To: Chen, Yu C
> > > > > > Cc: linux-pm@vger.kernel.org; edubezval@gmail.com; Zhang, Rui;
> > > > > > linux- kernel@vger.kernel.org; stable@vger.kernel.org
> > > > > > Subject: Re: [PATCH 3/3] Thermal: do thermal zone update after a
> > > > > > cooling device registered
> > > > > >
> > > > > > On Sun, Sep 27, 2015 at 06:48:44AM +0100, Chen Yu wrote:
> > > > > > > From: Zhang Rui <rui.zhang@intel.com>
> > > > > > >
> > > > > > >
> > > > > >
> > > > > > I think you need to hold cdev->lock here, to make sure that no
> > > > > > thermal zone is added or removed from cdev->thermal_instances
> > > > > > while
> > > > you are looping.
> > > > > >
> > > > > Ah right, will add. If I add the cdev ->lock here, will there be a
> > > > > AB-BA lock with thermal_zone_unbind_cooling_device?
> > > >
> > > > You're right, it could lead to a deadlock.  The locks can't be
> > > > swapped because that won't work in step_wise.
> > > >
> > > > The best way that I can think of accessing thermal_instances
> > > > atomically is by making it RCU protected instead of with mutexes.
> > > > What do you think?
> > > >
> > > RCU would need extra spinlocks to protect the list, and need to
> > > sync_rcu after we delete one instance from thermal_instance list,  I
> > > think it is too complicated for me to rewrite: ( How about using
> > thermal_list_lock instead of cdev ->lock?
> > > This guy should be big enough to protect the device.thermal_instance list.
> > 
> > thermal_list_lock protects thermal_tz_list and thermal_cdev_list, but it
> > doesn't protect the thermal_instances list.  For example,
> > thermal_zone_bind_cooling_device() adds a cooling device to the
> > cdev->thermal_instances list without taking thermal_tz_list.
> > 
> Before thermal_zone_bind_cooling_device is invoked,
> the thermal_list_lock will be firstly gripped:
> 
> static void bind_cdev(struct thermal_cooling_device *cdev)
> {
> mutex_lock(&thermal_list_lock);
> either tz->ops->bind    :   thermal_zone_bind_cooling_device
> or __bind()  :   thermal_zone_bind_cooling_device
> mutex_unlock(&thermal_list_lock);
> }
> 
> And it is the same as in  passive_store.
> So when code is trying to add/delete thermal_instance of cdev,
> he has already hold thermal_list_lock IMO. Or do I miss anything?

thermal_zone_bind_cooling_device() is exported, so you can't really
rely on the static thermal_list_lock being acquired in every single
call.

thermal_list_lock and protects the lists thermal_tz_list and
thermal_cdev_list.  Making it implicitly protect the cooling device's
and thermal zone device's instances list because no sensible code
would call thermal_zone_bind_cooling_device() outside of a bind
function is just asking for trouble.

Locking is hard to understand and easy to get wrong so let's keep it
simple.

Cheers,
Javi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247106

From"Chen, Yu C" <yu.c.chen@intel.com>
Date2015-10-14 21:30 +0200
Message-ID<qjzUm-83J-13@gated-at.bofh.it>
In reply to#1247009
SGkgSmF2aSwNCg0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IEphdmkg
TWVyaW5vIFttYWlsdG86amF2aS5tZXJpbm9AYXJtLmNvbV0NCj4gU2VudDogVGh1cnNkYXksIE9j
dG9iZXIgMTUsIDIwMTUgMTowOCBBTQ0KPiBUbzogQ2hlbiwgWXUgQw0KPiBDYzogbGludXgtcG1A
dmdlci5rZXJuZWwub3JnOyBlZHViZXp2YWxAZ21haWwuY29tOyBaaGFuZywgUnVpOyBsaW51eC0N
Cj4ga2VybmVsQHZnZXIua2VybmVsLm9yZzsgc3RhYmxlQHZnZXIua2VybmVsLm9yZzsgUGFuZHJ1
dmFkYSwgU3Jpbml2YXMNCj4gU3ViamVjdDogUmU6IFtQQVRDSCAzLzNdIFRoZXJtYWw6IGRvIHRo
ZXJtYWwgem9uZSB1cGRhdGUgYWZ0ZXIgYSBjb29saW5nDQo+IGRldmljZSByZWdpc3RlcmVkDQo+
IA0KPiBPbiBNb24sIE9jdCAxMiwgMjAxNSBhdCAwOToyMzoyOEFNICswMDAwLCBDaGVuLCBZdSBD
IHdyb3RlOg0KPiA+IEhpLCBKYXZpDQo+ID4gU29ycnkgZm9yIG15IGxhdGUgcmVzcG9uc2UsDQo+
ID4NCj4gPiA+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiBGcm9tOiBKYXZpIE1l
cmlubyBbbWFpbHRvOmphdmkubWVyaW5vQGFybS5jb21dDQo+ID4gPiBTZW50OiBXZWRuZXNkYXks
IFNlcHRlbWJlciAzMCwgMjAxNSAxMjowMiBBTQ0KPiA+ID4gVG86IENoZW4sIFl1IEMNCj4gPiA+
IENjOiBsaW51eC1wbUB2Z2VyLmtlcm5lbC5vcmc7IGVkdWJlenZhbEBnbWFpbC5jb207IFpoYW5n
LCBSdWk7DQo+ID4gPiBsaW51eC0ga2VybmVsQHZnZXIua2VybmVsLm9yZzsgc3RhYmxlQHZnZXIu
a2VybmVsLm9yZw0KPiA+ID4gU3ViamVjdDogUmU6IFtQQVRDSCAzLzNdIFRoZXJtYWw6IGRvIHRo
ZXJtYWwgem9uZSB1cGRhdGUgYWZ0ZXIgYQ0KPiA+ID4gY29vbGluZyBkZXZpY2UgcmVnaXN0ZXJl
ZA0KPiA+ID4NCj4gPiA+IEhpIFl1LA0KPiA+ID4NCj4gPiA+IE9uIE1vbiwgU2VwIDI4LCAyMDE1
IGF0IDA2OjUyOjAwUE0gKzAxMDAsIENoZW4sIFl1IEMgd3JvdGU6DQo+ID4gPiA+IEhpLCBKYXZp
LA0KPiA+ID4gPg0KPiA+ID4gPiA+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiA+
ID4gRnJvbTogSmF2aSBNZXJpbm8gW21haWx0bzpqYXZpLm1lcmlub0Bhcm0uY29tXQ0KPiA+ID4g
PiA+IFNlbnQ6IE1vbmRheSwgU2VwdGVtYmVyIDI4LCAyMDE1IDEwOjI5IFBNDQo+ID4gPiA+ID4g
VG86IENoZW4sIFl1IEMNCj4gPiA+ID4gPiBDYzogbGludXgtcG1Admdlci5rZXJuZWwub3JnOyBl
ZHViZXp2YWxAZ21haWwuY29tOyBaaGFuZywgUnVpOw0KPiA+ID4gPiA+IGxpbnV4LSBrZXJuZWxA
dmdlci5rZXJuZWwub3JnOyBzdGFibGVAdmdlci5rZXJuZWwub3JnDQo+ID4gPiA+ID4gU3ViamVj
dDogUmU6IFtQQVRDSCAzLzNdIFRoZXJtYWw6IGRvIHRoZXJtYWwgem9uZSB1cGRhdGUgYWZ0ZXIg
YQ0KPiA+ID4gPiA+IGNvb2xpbmcgZGV2aWNlIHJlZ2lzdGVyZWQNCj4gPiA+ID4gPg0KPiA+ID4g
PiA+IE9uIFN1biwgU2VwIDI3LCAyMDE1IGF0IDA2OjQ4OjQ0QU0gKzAxMDAsIENoZW4gWXUgd3Jv
dGU6DQo+ID4gPiA+ID4gPiBGcm9tOiBaaGFuZyBSdWkgPHJ1aS56aGFuZ0BpbnRlbC5jb20+DQo+
ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4NCj4gPiA+ID4gPg0KPiA+ID4gPiA+IEkgdGhpbmsgeW91
IG5lZWQgdG8gaG9sZCBjZGV2LT5sb2NrIGhlcmUsIHRvIG1ha2Ugc3VyZSB0aGF0IG5vDQo+ID4g
PiA+ID4gdGhlcm1hbCB6b25lIGlzIGFkZGVkIG9yIHJlbW92ZWQgZnJvbSBjZGV2LT50aGVybWFs
X2luc3RhbmNlcw0KPiA+ID4gPiA+IHdoaWxlDQo+ID4gPiB5b3UgYXJlIGxvb3BpbmcuDQo+ID4g
PiA+ID4NCj4gPiA+ID4gQWggcmlnaHQsIHdpbGwgYWRkLiBJZiBJIGFkZCB0aGUgY2RldiAtPmxv
Y2sgaGVyZSwgd2lsbCB0aGVyZSBiZSBhDQo+ID4gPiA+IEFCLUJBIGxvY2sgd2l0aCB0aGVybWFs
X3pvbmVfdW5iaW5kX2Nvb2xpbmdfZGV2aWNlPw0KPiA+ID4NCj4gPiA+IFlvdSdyZSByaWdodCwg
aXQgY291bGQgbGVhZCB0byBhIGRlYWRsb2NrLiAgVGhlIGxvY2tzIGNhbid0IGJlDQo+ID4gPiBz
d2FwcGVkIGJlY2F1c2UgdGhhdCB3b24ndCB3b3JrIGluIHN0ZXBfd2lzZS4NCj4gPiA+DQo+ID4g
PiBUaGUgYmVzdCB3YXkgdGhhdCBJIGNhbiB0aGluayBvZiBhY2Nlc3NpbmcgdGhlcm1hbF9pbnN0
YW5jZXMNCj4gPiA+IGF0b21pY2FsbHkgaXMgYnkgbWFraW5nIGl0IFJDVSBwcm90ZWN0ZWQgaW5z
dGVhZCBvZiB3aXRoIG11dGV4ZXMuDQo+ID4gPiBXaGF0IGRvIHlvdSB0aGluaz8NCj4gPiA+DQo+
ID4gUkNVIHdvdWxkIG5lZWQgZXh0cmEgc3BpbmxvY2tzIHRvIHByb3RlY3QgdGhlIGxpc3QsIGFu
ZCBuZWVkIHRvDQo+ID4gc3luY19yY3UgYWZ0ZXIgd2UgZGVsZXRlIG9uZSBpbnN0YW5jZSBmcm9t
IHRoZXJtYWxfaW5zdGFuY2UgbGlzdCwgIEkNCj4gPiB0aGluayBpdCBpcyB0b28gY29tcGxpY2F0
ZWQgZm9yIG1lIHRvIHJld3JpdGU6ICggSG93IGFib3V0IHVzaW5nDQo+IHRoZXJtYWxfbGlzdF9s
b2NrIGluc3RlYWQgb2YgY2RldiAtPmxvY2s/DQo+ID4gVGhpcyBndXkgc2hvdWxkIGJlIGJpZyBl
bm91Z2ggdG8gcHJvdGVjdCB0aGUgZGV2aWNlLnRoZXJtYWxfaW5zdGFuY2UgbGlzdC4NCj4gDQo+
IHRoZXJtYWxfbGlzdF9sb2NrIHByb3RlY3RzIHRoZXJtYWxfdHpfbGlzdCBhbmQgdGhlcm1hbF9j
ZGV2X2xpc3QsIGJ1dCBpdA0KPiBkb2Vzbid0IHByb3RlY3QgdGhlIHRoZXJtYWxfaW5zdGFuY2Vz
IGxpc3QuICBGb3IgZXhhbXBsZSwNCj4gdGhlcm1hbF96b25lX2JpbmRfY29vbGluZ19kZXZpY2Uo
KSBhZGRzIGEgY29vbGluZyBkZXZpY2UgdG8gdGhlDQo+IGNkZXYtPnRoZXJtYWxfaW5zdGFuY2Vz
IGxpc3Qgd2l0aG91dCB0YWtpbmcgdGhlcm1hbF90el9saXN0Lg0KPiANCg0KQmVmb3JlIHRoZXJt
YWxfem9uZV9iaW5kX2Nvb2xpbmdfZGV2aWNlIGlzIGludm9rZWQsIA0KdGhlIHRoZXJtYWxfbGlz
dF9sb2NrIHdpbGwgYmUgZmlyc3RseSBncmlwcGVkOg0KDQpzdGF0aWMgdm9pZCBiaW5kX2NkZXYo
c3RydWN0IHRoZXJtYWxfY29vbGluZ19kZXZpY2UgKmNkZXYpDQp7DQoNCm11dGV4X2xvY2soJnRo
ZXJtYWxfbGlzdF9sb2NrKTsNCg0KZWl0aGVyIHR6LT5vcHMtPmJpbmQgICAgOiAgIHRoZXJtYWxf
em9uZV9iaW5kX2Nvb2xpbmdfZGV2aWNlDQoNCm9yIF9fYmluZCgpICA6ICAgdGhlcm1hbF96b25l
X2JpbmRfY29vbGluZ19kZXZpY2UNCm11dGV4X3VubG9jaygmdGhlcm1hbF9saXN0X2xvY2spOw0K
DQp9DQoNCkFuZCBpdCBpcyB0aGUgc2FtZSBhcyBpbiAgcGFzc2l2ZV9zdG9yZS4NCg0KU28gd2hl
biBjb2RlIGlzIHRyeWluZyB0byBhZGQvZGVsZXRlIHRoZXJtYWxfaW5zdGFuY2Ugb2YgY2Rldiwg
aGUgaGFzDQphbHJlYWR5IGhvbGQgdGhlcm1hbF9saXN0X2xvY2sgSU1PLiBPciBkbyBJIG1pc3Mg
YW55dGhpbmc/DQoNCkJlc3QgUmVnYXJkcywNCll1DQoNCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web