Skip to contentSkip to conversation

srp_daemon: Remove duplicate sync_res shutdown

PATCH Superseded Jason Gunthorpe 2 participants 4 messages Dec 19, 2017 3 messages not in the archive InfiniBand

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Jason Gunthorpe

author
#0 lore raw

PATCH 1/1 srp_daemon: Remove duplicate sync_res shutdown

The very first thing free_res does is the same lock/signal/unlock sequence, so there is no reason to open code it before calling free_res.

Signed-off-by Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/[email protected]>
---
 srp_daemon/srp_daemon.c | 4 ----
 1 file changed, 4 deletions(-)

Noticed while looking at honli's patch.. Didn't make a PR.

srp_daemon/srp_daemon.c +0 -4
index cec36db2e0f12e..a1798558373490 100644
@@ -2074,10 +2074,6 @@ static int ibsrpdm(int argc, char *argv[])
 		pr_err("Querying SRP targets failed\n");
 
 	assert(res->sync_res);
-	pthread_mutex_lock(&res->sync_res->retry_mutex);
-	res->sync_res->stop_threads = 1;
-	pthread_cond_signal(&res->sync_res->retry_cond);
-	pthread_mutex_unlock(&res->sync_res->retry_mutex);
 
 	free_res(res);
 umad_done:
Signature · 6 lines
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20171219211601.GA27412-uk2M96/[email protected]

#2 lore raw

T24gVHVlLCAyMDE3LTEyLTE5IGF0IDE0OjE2IC0wNzAwLCBKYXNvbiBHdW50aG9ycGUgd3JvdGU6 DQo+IFRoZSB2ZXJ5IGZpcnN0IHRoaW5nIGZyZWVfcmVzIGRvZXMgaXMgdGhlIHNhbWUgbG9jay9z aWduYWwvdW5sb2NrDQo+IHNlcXVlbmNlLCBzbyB0aGVyZSBpcyBubyByZWFzb24gdG8gb3BlbiBj b2RlIGl0IGJlZm9yZSBjYWxsaW5nDQo+IGZyZWVfcmVzLg0KPiANCj4gU2lnbmVkLW9mZi1ieTog SmFzb24gR3VudGhvcnBlIDxqZ2dAbWVsbGFub3guY29tPg0KPiAtLS0NCj4gIHNycF9kYWVtb24v c3JwX2RhZW1vbi5jIHwgNCAtLS0tDQo+ICAxIGZpbGUgY2hhbmdlZCwgNCBkZWxldGlvbnMoLSkN Cj4gDQo+IE5vdGljZWQgd2hpbGUgbG9va2luZyBhdCBob25saSdzIHBhdGNoLi4gRGlkbid0IG1h a2UgYSBQUi4NCj4gDQo+IGRpZmYgLS1naXQgYS9zcnBfZGFlbW9uL3NycF9kYWVtb24uYyBiL3Ny cF9kYWVtb24vc3JwX2RhZW1vbi5jDQo+IGluZGV4IGNlYzM2ZGIyZTBmMTJlLi5hMTc5ODU1ODM3 MzQ5MCAxMDA2NDQNCj4gLS0tIGEvc3JwX2RhZW1vbi9zcnBfZGFlbW9uLmMNCj4gKysrIGIvc3Jw X2RhZW1vbi9zcnBfZGFlbW9uLmMNCj4gQEAgLTIwNzQsMTAgKzIwNzQsNiBAQCBzdGF0aWMgaW50 IGlic3JwZG0oaW50IGFyZ2MsIGNoYXIgKmFyZ3ZbXSkNCj4gIAkJcHJfZXJyKCJRdWVyeWluZyBT UlAgdGFyZ2V0cyBmYWlsZWRcbiIpOw0KPiAgDQo+ICAJYXNzZXJ0KHJlcy0+c3luY19yZXMpOw0K PiAtCXB0aHJlYWRfbXV0ZXhfbG9jaygmcmVzLT5zeW5jX3Jlcy0+cmV0cnlfbXV0ZXgpOw0KPiAt CXJlcy0+c3luY19yZXMtPnN0b3BfdGhyZWFkcyA9IDE7DQo+IC0JcHRocmVhZF9jb25kX3NpZ25h bCgmcmVzLT5zeW5jX3Jlcy0+cmV0cnlfY29uZCk7DQo+IC0JcHRocmVhZF9tdXRleF91bmxvY2so JnJlcy0+c3luY19yZXMtPnJldHJ5X211dGV4KTsNCg0KSGVsbG8gSmFzb24sDQoNClNpbmNlIHRo aXMgcGF0Y2ggcmVtb3ZlcyBhbGwgY29kZSB0aGF0IGRlcmVmZXJlbmNlcyByZXMtPnN5bmNfcmVz IGZyb20NCnRoZSBpYnNycGRtKCkgZnVuY3Rpb24sIGhhdmUgeW91IGNvbnNpZGVyZWQgdG8gcmVt b3ZlIGFzc2VydChyZXMtPnN5bmNfcmVzKQ0KdG9vPw0KDQpUaGFua3MsDQoNCkJhcnQu --
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#4 lore raw
Quoted 25 lines · Bart Van Assche
Quoted 14 lines · Jason Gunthorpe

The very first thing free_res does is the same lock/signal/unlock sequence, so there is no reason to open code it before calling free_res.

Signed-off-by Jason Gunthorpe <jgg-VPRAkNaXOzVWk0Htik3J/[email protected]>
 srp_daemon/srp_daemon.c | 4 ----
 1 file changed, 4 deletions(-)

Noticed while looking at honli's patch.. Didn't make a PR.

srp_daemon/srp_daemon.c +1 -0
index cec36db2e0f12e..a1798558373490 100644
+++ b/srp_daemon/srp_daemon.c

srp_daemon/srp_daemon.c static int ibsrpdm(int argc, char *argv[]) · line 2080

Quoted 7 lines
srp_daemon/srp_daemon.c +0 -4
@@ -2074,10 +2074,6 @@ static int ibsrpdm(int argc, char *argv[])
 		pr_err("Querying SRP targets failed\n");
 
 	assert(res->sync_res);
-	pthread_mutex_lock(&res->sync_res->retry_mutex);
-	res->sync_res->stop_threads = 1;
-	pthread_cond_signal(&res->sync_res->retry_cond);
-	pthread_mutex_unlock(&res->sync_res->retry_mutex);

Hello Jason,

Quoted 3 lines

Since this patch removes all code that dereferences res->sync_res from the ibsrpdm() function, have you considered to remove assert(res->sync_res) too?

Well, that would be a behavior change.. free_res works OK if sync_res is NULL, so I'm guessing that the assert had some meaning to show that the reconnect_thread was always started for ibsrpd?

Or maybe we shouldn't care about that?

Happy to drop it if you think it makes sense.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20171219212818.GG14800-VPRAkNaXOzVWk0Htik3J/[email protected]

#6 lore raw

T24gVHVlLCAyMDE3LTEyLTE5IGF0IDE0OjI4IC0wNzAwLCBKYXNvbiBHdW50aG9ycGUgd3JvdGU6 DQo+IE9uIFR1ZSwgRGVjIDE5LCAyMDE3IGF0IDA5OjIxOjA1UE0gKzAwMDAsIEJhcnQgVmFuIEFz c2NoZSB3cm90ZToNCj4gPiBPbiBUdWUsIDIwMTctMTItMTkgYXQgMTQ6MTYgLTA3MDAsIEphc29u IEd1bnRob3JwZSB3cm90ZToNCj4gPiA+IFRoZSB2ZXJ5IGZpcnN0IHRoaW5nIGZyZWVfcmVzIGRv ZXMgaXMgdGhlIHNhbWUgbG9jay9zaWduYWwvdW5sb2NrDQo+ID4gPiBzZXF1ZW5jZSwgc28gdGhl cmUgaXMgbm8gcmVhc29uIHRvIG9wZW4gY29kZSBpdCBiZWZvcmUgY2FsbGluZw0KPiA+ID4gZnJl ZV9yZXMuDQo+ID4gPiANCj4gPiA+IFNpZ25lZC1vZmYtYnk6IEphc29uIEd1bnRob3JwZSA8amdn QG1lbGxhbm94LmNvbT4NCj4gPiA+ICBzcnBfZGFlbW9uL3NycF9kYWVtb24uYyB8IDQgLS0tLQ0K PiA+ID4gIDEgZmlsZSBjaGFuZ2VkLCA0IGRlbGV0aW9ucygtKQ0KPiA+ID4gDQo+ID4gPiBOb3Rp Y2VkIHdoaWxlIGxvb2tpbmcgYXQgaG9ubGkncyBwYXRjaC4uIERpZG4ndCBtYWtlIGEgUFIuDQo+ ID4gPiANCj4gPiA+IGRpZmYgLS1naXQgYS9zcnBfZGFlbW9uL3NycF9kYWVtb24uYyBiL3NycF9k YWVtb24vc3JwX2RhZW1vbi5jDQo+ID4gPiBpbmRleCBjZWMzNmRiMmUwZjEyZS4uYTE3OTg1NTgz NzM0OTAgMTAwNjQ0DQo+ID4gPiArKysgYi9zcnBfZGFlbW9uL3NycF9kYWVtb24uYw0KPiA+ID4g QEAgLTIwNzQsMTAgKzIwNzQsNiBAQCBzdGF0aWMgaW50IGlic3JwZG0oaW50IGFyZ2MsIGNoYXIg KmFyZ3ZbXSkNCj4gPiA+ICAJCXByX2VycigiUXVlcnlpbmcgU1JQIHRhcmdldHMgZmFpbGVkXG4i KTsNCj4gPiA+ICANCj4gPiA+ICAJYXNzZXJ0KHJlcy0+c3luY19yZXMpOw0KPiA+ID4gLQlwdGhy ZWFkX211dGV4X2xvY2soJnJlcy0+c3luY19yZXMtPnJldHJ5X211dGV4KTsNCj4gPiA+IC0JcmVz LT5zeW5jX3Jlcy0+c3RvcF90aHJlYWRzID0gMTsNCj4gPiA+IC0JcHRocmVhZF9jb25kX3NpZ25h bCgmcmVzLT5zeW5jX3Jlcy0+cmV0cnlfY29uZCk7DQo+ID4gPiAtCXB0aHJlYWRfbXV0ZXhfdW5s b2NrKCZyZXMtPnN5bmNfcmVzLT5yZXRyeV9tdXRleCk7DQo+ID4gDQo+ID4gSGVsbG8gSmFzb24s DQo+ID4gDQo+ID4gU2luY2UgdGhpcyBwYXRjaCByZW1vdmVzIGFsbCBjb2RlIHRoYXQgZGVyZWZl cmVuY2VzIHJlcy0+c3luY19yZXMgZnJvbQ0KPiA+IHRoZSBpYnNycGRtKCkgZnVuY3Rpb24sIGhh dmUgeW91IGNvbnNpZGVyZWQgdG8gcmVtb3ZlIGFzc2VydChyZXMtPnN5bmNfcmVzKQ0KPiA+IHRv bz8NCj4gDQo+IFdlbGwsIHRoYXQgd291bGQgYmUgYSBiZWhhdmlvciBjaGFuZ2UuLiBmcmVlX3Jl cyB3b3JrcyBPSyBpZiBzeW5jX3Jlcw0KPiBpcyBOVUxMLCBzbyBJJ20gZ3Vlc3NpbmcgdGhhdCB0 aGUgYXNzZXJ0IGhhZCBzb21lIG1lYW5pbmcgdG8gc2hvdyB0aGF0DQo+IHRoZSByZWNvbm5lY3Rf dGhyZWFkIHdhcyBhbHdheXMgc3RhcnRlZCBmb3IgaWJzcnBkPw0KPiANCj4gT3IgbWF5YmUgd2Ug c2hvdWxkbid0IGNhcmUgYWJvdXQgdGhhdD8NCj4gDQo+IEhhcHB5IHRvIGRyb3AgaXQgaWYgeW91 IHRoaW5rIGl0IG1ha2VzIHNlbnNlLg0KDQpIZWxsbyBKYXNvbiwNCg0KSWYgdGhlIGFsbG9jX3Jl cygpIHJldHVybiB2YWx1ZSBpcyBub3QgTlVMTCB0aGVuIHRoYXQgaW1wbGllcyB0aGF0IHJlcy0+ c3luY19yZXMNCiE9IE5VTEwuIFRoZSBhc3NlcnQoKSBJIHJlZmVycmVkIHRvIGlzIG9ubHkgcmVh Y2hlZCBpZiBhbGxvY19yZXMoKSBzdWNjZWVkZWQuIFNvDQpJIHRoaW5rIGl0IGl0IGlzIHNhZmUg dG8gbGVhdmUgb3V0IHRoYXQgYXNzZXJ0KCkgc3RhdGVtZW50Lg0KDQpCYXJ0Lg0KDQo= --
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html