Skip to contentSkip to conversation

SRP fail-over faster

PATCH In review Vu Pham 2 participants 4 messages Oct 12, 2009 – Oct 22, 2009 2 messages not in the archive

Subsystems (2)
lore
Share & tools
lore

Vu Pham

author
#0 lore raw

PATCH 4/4 SRP fail-over faster

Handling async local port events: port err, active, lid change...
Upon local port err, it will set up timer srp_dev_loss_tmo seconds to reconnect. If local port active and there is timer, it will delete the timer

drivers/infiniband/ulp/srp/ib_srp.c +78 -0
Index: ofed_kernel/drivers/infiniband/ulp/srp/ib_srp.c
===================================================================
@@ -2071,6 +2120,77 @@
 	return NULL;
 }
 
+static void srp_event_handler(struct ib_event_handler *handler,
+			      struct ib_event *event)
+{
+	struct srp_device *srp_dev =
+	    ib_get_client_data(event->device, &srp_client);
+	struct srp_host *host, *tmp_host;
+	struct srp_target_port *target, *tmp_target;
+
+	if (!srp_dev || srp_dev->dev != event->device)
+		return;
+
+	printk(KERN_WARNING PFX "ASYNC event= %d on device= %s\n",
+		event->event, srp_dev->dev->name);
+
+	switch (event->event) {
+	case IB_EVENT_PORT_ERR:
+		list_for_each_entry_safe(host, tmp_host,
+					 &srp_dev->dev_list, list) {
+			if (event->element.port_num == host->port) {
+				spin_lock(&host->target_lock);
+				list_for_each_entry_safe(target, tmp_target,
+							 &host->target_list, list) {
+					unsigned long flags;
+
+					spin_lock_irqsave(target->scsi_host->host_lock,
+							  flags);
+					if (!target->qp_in_error &&
+					    target->state == SRP_TARGET_LIVE)
+						srp_qp_err_add_timer(target,
+								     srp_dev_loss_tmo);
+					spin_unlock_irqrestore(target->scsi_host->host_lock,
+							       flags);
+				}
+				spin_unlock(&host->target_lock);
+			}
+		}
+		break;
+	case IB_EVENT_PORT_ACTIVE:
+	case IB_EVENT_LID_CHANGE:
+	case IB_EVENT_PKEY_CHANGE:
+	case IB_EVENT_SM_CHANGE:
+		list_for_each_entry_safe(host, tmp_host, &srp_dev->dev_list,
+					 list) {
+			if (event->element.port_num == host->port) {
+				spin_lock(&host->target_lock);
+				list_for_each_entry_safe(target, tmp_target,
+							 &host->target_list, list) {
+					unsigned long flags;
+
+					spin_lock_irqsave(target->scsi_host->host_lock,
+							  flags);
+					if (timer_pending(&target->qp_err_timer)
+					    && !target->qp_in_error) {
+						shost_printk(KERN_WARNING PFX,
+							     target->scsi_host,
+							     "delete qp_in_err timer\n");
+						del_timer(&target->qp_err_timer);
+					}
+					spin_unlock_irqrestore(target->scsi_host->host_lock,
+							       flags);
+				}
+				spin_unlock(&host->target_lock);
+			}
+		}
+		break;
+	default:
+		break;
+	}
+
+}
+
 static void srp_add_one(struct ib_device *device)
 {
 	struct srp_device *srp_dev;
@@ -2116,6 +2228,11 @@
 	if (IS_ERR(srp_dev->mr))
 		goto err_pd;
 
+	INIT_IB_EVENT_HANDLER(&srp_dev->event_handler, srp_dev->dev,
+			      srp_event_handler);
+	if (ib_register_event_handler(&srp_dev->event_handler))
+		goto err_pd;
+
 	memset(&fmr_param, 0, sizeof fmr_param);
 	fmr_param.pool_size	    = SRP_FMR_POOL_SIZE;
 	fmr_param.dirty_watermark   = SRP_FMR_DIRTY_SIZE;
@@ -2160,6 +2284,8 @@
 
 	srp_dev = ib_get_client_data(device, &srp_client);
 
+	ib_unregister_event_handler(&srp_dev->event_handler);
+
 	list_for_each_entry_safe(host, tmp_host, &srp_dev->dev_list, list) {
 		device_unregister(&host->dev);
 		/*
drivers/infiniband/ulp/srp/ib_srp.h +1 -0
Index: ofed_kernel/drivers/infiniband/ulp/srp/ib_srp.h
===================================================================
@@ -88,6 +88,7 @@ struct srp_device {
 	struct ib_device       *dev;
 	struct ib_pd	       *pd;
 	struct ib_mr	       *mr;
+	struct ib_event_handler event_handler;
 	struct ib_fmr_pool     *fmr_pool;
 	int			fmr_page_shift;
 	int			fmr_page_size;

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

#2 lore raw

T24gVHVlLCBPY3QgMTMsIDIwMDkgYXQgMTI6NTcgQU0sIFZ1IFBoYW0gPHZ1aHVvbmdAbWVsbGFu b3guY29tPiB3cm90ZToKPgo+Cj4gSGFuZGxpbmcgYXN5bmMgbG9jYWwgcG9ydCBldmVudHM6IHBv cnQgZXJyLCBhY3RpdmUsIGxpZCBjaGFuZ2UuLi4KPiBVcG9uIGxvY2FsIHBvcnQgZXJyLCBpdCB3 aWxsIHNldCB1cCB0aW1lciBzcnBfZGV2X2xvc3NfdG1vIHNlY29uZHMgdG8gcmVjb25uZWN0LiBJ ZiBsb2NhbCBwb3J0IGFjdGl2ZSBhbmQgdGhlcmUgaXMgdGltZXIsIGl0IHdpbGwgZGVsZXRlIHRo ZSB0aW1lcgo+Cj4gU2lnbmVkLW9mZi1ieTogVnUgUGhhbSA8dnVAbWVsbGFub3guY29tPgo+Cj4g SW5kZXg6IG9mZWRfa2VybmVsL2RyaXZlcnMvaW5maW5pYmFuZC91bHAvc3JwL2liX3NycC5jCj4g PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQo+IC0tLSBvZmVkX2tlcm5lbC5vcmlnL2RyaXZlcnMvaW5maW5pYmFuZC91bHAv c3JwL2liX3NycC5jCj4gKysrIG9mZWRfa2VybmVsL2RyaXZlcnMvaW5maW5pYmFuZC91bHAvc3Jw L2liX3NycC5jCj4gQEAgLTIwNzEsNiArMjEyMCw3NyBAQAo+IKAgoCCgIKByZXR1cm4gTlVMTDsK PiCgfQo+Cj4gK3N0YXRpYyB2b2lkIHNycF9ldmVudF9oYW5kbGVyKHN0cnVjdCBpYl9ldmVudF9o YW5kbGVyICpoYW5kbGVyLAo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIHN0cnVjdCBp Yl9ldmVudCAqZXZlbnQpCj4gK3sKPiArIKAgoCCgIHN0cnVjdCBzcnBfZGV2aWNlICpzcnBfZGV2 ID0KPiArIKAgoCCgIKAgoCBpYl9nZXRfY2xpZW50X2RhdGEoZXZlbnQtPmRldmljZSwgJnNycF9j bGllbnQpOwo+ICsgoCCgIKAgc3RydWN0IHNycF9ob3N0ICpob3N0LCAqdG1wX2hvc3Q7Cj4gKyCg IKAgoCBzdHJ1Y3Qgc3JwX3RhcmdldF9wb3J0ICp0YXJnZXQsICp0bXBfdGFyZ2V0Owo+ICsKPiAr IKAgoCCgIGlmICghc3JwX2RldiB8fCBzcnBfZGV2LT5kZXYgIT0gZXZlbnQtPmRldmljZSkKPiAr IKAgoCCgIKAgoCCgIKAgcmV0dXJuOwo+ICsKPiArIKAgoCCgIHByaW50ayhLRVJOX1dBUk5JTkcg UEZYICJBU1lOQyBldmVudD0gJWQgb24gZGV2aWNlPSAlc1xuIiwKPiArIKAgoCCgIKAgoCCgIKAg ZXZlbnQtPmV2ZW50LCBzcnBfZGV2LT5kZXYtPm5hbWUpOwo+ICsKPiArIKAgoCCgIHN3aXRjaCAo ZXZlbnQtPmV2ZW50KSB7Cj4gKyCgIKAgoCBjYXNlIElCX0VWRU5UX1BPUlRfRVJSOgo+ICsgoCCg IKAgoCCgIKAgoCBsaXN0X2Zvcl9lYWNoX2VudHJ5X3NhZmUoaG9zdCwgdG1wX2hvc3QsCj4gKyCg IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAmc3JwX2Rldi0+ZGV2X2xpc3Qs IGxpc3QpIHsKPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCBpZiAoZXZlbnQtPmVsZW1lbnQucG9y dF9udW0gPT0gaG9zdC0+cG9ydCkgewo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAg c3Bpbl9sb2NrKCZob3N0LT50YXJnZXRfbG9jayk7CgpDYW4gc3JwX3JlbW92ZV93b3JrKCkgYmUg ZXhlY3V0ZWQgY29uY3VycmVudGx5IHdpdGgKc3JwX2V2ZW50X2hhbmRsZXIoKSA/IEluIHRoYXQg Y2FzZSB0aGUgYWJvdmUgY29kZSBpc24ndCBzYWZlIGFuZCB0aGUKc3Bpbl9sb2NrKCZob3N0LT50 YXJnZXRfbG9jaykgc2hvdWxkIGJlIG1vdmVkIHRvIGp1c3QgYmVmb3JlCmxpc3RfZm9yX2VhY2hf ZW50cnlfc2FmZSgpLiBUaGUgY3VycmVudCBpbXBsZW1lbnRhdGlvbiBjYW4gdHJpZ2dlcgpyZWFk aW5nIGRlYWxsb2NhdGVkIG1lbW9yeS4KCj4gKyCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAg oCBsaXN0X2Zvcl9lYWNoX2VudHJ5X3NhZmUodGFyZ2V0LCB0bXBfdGFyZ2V0LAo+ICsgoCCgIKAg oCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCZob3N0LT50 YXJnZXRfbGlzdCwgbGlzdCkgewo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCg IKAgoCB1bnNpZ25lZCBsb25nIGZsYWdzOwo+ICsKPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCg IKAgoCCgIKAgoCCgIKAgc3Bpbl9sb2NrX2lycXNhdmUodGFyZ2V0LT5zY3NpX2hvc3QtPmhvc3Rf bG9jaywKPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCg IKAgoCCgIKAgZmxhZ3MpOwo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAg oCBpZiAoIXRhcmdldC0+cXBfaW5fZXJyb3IgJiYKPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCg IKAgoCCgIKAgoCCgIKAgoCCgIHRhcmdldC0+c3RhdGUgPT0gU1JQX1RBUkdFVF9MSVZFKQo+ICsg oCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIHNycF9xcF9lcnJf YWRkX3RpbWVyKHRhcmdldCwKPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCg IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKBzcnBfZGV2X2xvc3NfdG1vKTsKPiArIKAg oCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgc3Bpbl91bmxvY2tfaXJxcmVzdG9y ZSh0YXJnZXQtPnNjc2lfaG9zdC0+aG9zdF9sb2NrLAo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCg IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoGZsYWdzKTsKPiArIKAgoCCg IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIH0KPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAg oCCgIHNwaW5fdW5sb2NrKCZob3N0LT50YXJnZXRfbG9jayk7Cj4gKyCgIKAgoCCgIKAgoCCgIKAg oCCgIKAgfQo+ICsgoCCgIKAgoCCgIKAgoCB9Cj4gKyCgIKAgoCCgIKAgoCCgIGJyZWFrOwo+ICsg oCCgIKAgY2FzZSBJQl9FVkVOVF9QT1JUX0FDVElWRToKPiArIKAgoCCgIGNhc2UgSUJfRVZFTlRf TElEX0NIQU5HRToKPiArIKAgoCCgIGNhc2UgSUJfRVZFTlRfUEtFWV9DSEFOR0U6Cj4gKyCgIKAg oCBjYXNlIElCX0VWRU5UX1NNX0NIQU5HRToKPiArIKAgoCCgIKAgoCCgIKAgbGlzdF9mb3JfZWFj aF9lbnRyeV9zYWZlKGhvc3QsIHRtcF9ob3N0LCAmc3JwX2Rldi0+ZGV2X2xpc3QsCj4gKyCgIKAg oCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKBsaXN0KSB7Cj4gKyCgIKAgoCCgIKAg oCCgIKAgoCCgIKAgaWYgKGV2ZW50LT5lbGVtZW50LnBvcnRfbnVtID09IGhvc3QtPnBvcnQpIHsK PiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIHNwaW5fbG9jaygmaG9zdC0+dGFyZ2V0 X2xvY2spOwoKQSByZW1hcmsgc2ltaWxhciB0byB0aGUgYWJvdmU6IHRoaXMgY29kZSBpc24ndCBz YWZlIGFnYWluc3QgY29uY3VycmVudApjYWxscyBvZiBzcnBfcmVtb3ZlX29uZSgpLgoKPiArIKAg oCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIGxpc3RfZm9yX2VhY2hfZW50cnlfc2FmZSh0YXJn ZXQsIHRtcF90YXJnZXQsCj4gKyCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCg IKAgoCCgIKAgoCCgIKAgoCCgJmhvc3QtPnRhcmdldF9saXN0LCBsaXN0KSB7Cj4gKyCgIKAgoCCg IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIHVuc2lnbmVkIGxvbmcgZmxhZ3M7Cj4gKwo+ ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCBzcGluX2xvY2tfaXJxc2F2 ZSh0YXJnZXQtPnNjc2lfaG9zdC0+aG9zdF9sb2NrLAo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCg IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCBmbGFncyk7Cj4gKyCgIKAgoCCgIKAg oCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIGlmICh0aW1lcl9wZW5kaW5nKCZ0YXJnZXQtPnFw X2Vycl90aW1lcikKPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCg ICYmICF0YXJnZXQtPnFwX2luX2Vycm9yKSB7Cj4gKyCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCg IKAgoCCgIKAgoCCgIKAgoCCgIKAgc2hvc3RfcHJpbnRrKEtFUk5fV0FSTklORyBQRlgsCj4gKyCg IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAg oHRhcmdldC0+c2NzaV9ob3N0LAo+ICsgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCg IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAiZGVsZXRlIHFwX2luX2VyciB0aW1lclxuIik7Cj4g KyCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgZGVsX3RpbWVy KCZ0YXJnZXQtPnFwX2Vycl90aW1lcik7Cj4gKyCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAg oCCgIKAgoCCgIH0KPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgc3Bp bl91bmxvY2tfaXJxcmVzdG9yZSh0YXJnZXQtPnNjc2lfaG9zdC0+aG9zdF9sb2NrLAo+ICsgoCCg IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAg oGZsYWdzKTsKPiArIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIH0KPiArIKAgoCCgIKAg oCCgIKAgoCCgIKAgoCCgIKAgoCCgIHNwaW5fdW5sb2NrKCZob3N0LT50YXJnZXRfbG9jayk7Cj4g KyCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgfQo+ICsgoCCgIKAgoCCgIKAgoCB9Cj4gKyCgIKAgoCCg IKAgoCCgIGJyZWFrOwo+ICsgoCCgIKAgZGVmYXVsdDoKPiArIKAgoCCgIKAgoCCgIKAgYnJlYWs7 Cj4gKyCgIKAgoCB9Cj4gKwo+ICt9Cj4gKwo+IKBzdGF0aWMgdm9pZCBzcnBfYWRkX29uZShzdHJ1 Y3QgaWJfZGV2aWNlICpkZXZpY2UpCj4goHsKPiCgIKAgoCCgc3RydWN0IHNycF9kZXZpY2UgKnNy cF9kZXY7Cj4gQEAgLTIxMTYsNiArMjIyOCwxMSBAQAo+IKAgoCCgIKBpZiAoSVNfRVJSKHNycF9k ZXYtPm1yKSkKPiCgIKAgoCCgIKAgoCCgIKBnb3RvIGVycl9wZDsKPgo+ICsgoCCgIKAgSU5JVF9J Ql9FVkVOVF9IQU5ETEVSKCZzcnBfZGV2LT5ldmVudF9oYW5kbGVyLCBzcnBfZGV2LT5kZXYsCj4g KyCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgc3JwX2V2ZW50X2hhbmRsZXIpOwo+ICsgoCCg IKAgaWYgKGliX3JlZ2lzdGVyX2V2ZW50X2hhbmRsZXIoJnNycF9kZXYtPmV2ZW50X2hhbmRsZXIp KQo+ICsgoCCgIKAgoCCgIKAgoCBnb3RvIGVycl9wZDsKPiArCj4goCCgIKAgoG1lbXNldCgmZm1y X3BhcmFtLCAwLCBzaXplb2YgZm1yX3BhcmFtKTsKPiCgIKAgoCCgZm1yX3BhcmFtLnBvb2xfc2l6 ZSCgIKAgoCCgID0gU1JQX0ZNUl9QT09MX1NJWkU7Cj4goCCgIKAgoGZtcl9wYXJhbS5kaXJ0eV93 YXRlcm1hcmsgoCA9IFNSUF9GTVJfRElSVFlfU0laRTsKPiBAQCAtMjE2MCw2ICsyMjg0LDggQEAK Pgo+IKAgoCCgIKBzcnBfZGV2ID0gaWJfZ2V0X2NsaWVudF9kYXRhKGRldmljZSwgJnNycF9jbGll bnQpOwo+Cj4gKyCgIKAgoCBpYl91bnJlZ2lzdGVyX2V2ZW50X2hhbmRsZXIoJnNycF9kZXYtPmV2 ZW50X2hhbmRsZXIpOwo+ICsKPiCgIKAgoCCgbGlzdF9mb3JfZWFjaF9lbnRyeV9zYWZlKGhvc3Qs IHRtcF9ob3N0LCAmc3JwX2Rldi0+ZGV2X2xpc3QsIGxpc3QpIHsKPiCgIKAgoCCgIKAgoCCgIKBk ZXZpY2VfdW5yZWdpc3RlcigmaG9zdC0+ZGV2KTsKPiCgIKAgoCCgIKAgoCCgIKAvKgo+IEluZGV4 OiBvZmVkX2tlcm5lbC9kcml2ZXJzL2luZmluaWJhbmQvdWxwL3NycC9pYl9zcnAuaAo+ID09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KPiAtLS0gb2ZlZF9rZXJuZWwub3JpZy9kcml2ZXJzL2luZmluaWJhbmQvdWxwL3NycC9p Yl9zcnAuaAo+ICsrKyBvZmVkX2tlcm5lbC9kcml2ZXJzL2luZmluaWJhbmQvdWxwL3NycC9pYl9z cnAuaAo+IEBAIC04OCw2ICs4OCw3IEBAIHN0cnVjdCBzcnBfZGV2aWNlIHsKPiCgIKAgoCCgc3Ry dWN0IGliX2RldmljZSCgIKAgoCAqZGV2Owo+IKAgoCCgIKBzdHJ1Y3QgaWJfcGQgoCCgIKAgoCCg ICpwZDsKPiCgIKAgoCCgc3RydWN0IGliX21yIKAgoCCgIKAgoCAqbXI7Cj4gKyCgIKAgoCBzdHJ1 Y3QgaWJfZXZlbnRfaGFuZGxlciBldmVudF9oYW5kbGVyOwo+IKAgoCCgIKBzdHJ1Y3QgaWJfZm1y X3Bvb2wgoCCgICpmbXJfcG9vbDsKPiCgIKAgoCCgaW50IKAgoCCgIKAgoCCgIKAgoCCgIKAgZm1y X3BhZ2Vfc2hpZnQ7Cj4goCCgIKAgoGludCCgIKAgoCCgIKAgoCCgIKAgoCCgIGZtcl9wYWdlX3Np emU7Cj4K
--
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): e2e108260910130409q78e3edbcndd64b7cf419705b9-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

author
#4 lore raw
Quoted 44 lines · Bart Van Assche

drivers/infiniband/ulp/srp/ib_srp.c line 2142

Quoted 9 lines
drivers/infiniband/ulp/srp/ib_srp.c +7 -0
+
+       switch (event->event) {
+       case IB_EVENT_PORT_ERR:
+               list_for_each_entry_safe(host, tmp_host,
+                                        &srp_dev->dev_list, list) {
+                       if (event->element.port_num == host->port) {
+                               spin_lock(&host->target_lock);

Can srp_remove_work() be executed concurrently with srp_event_handler() ? In that case the above code isn't safe and the spin_lock(&host->target_lock) should be moved to just before list_for_each_entry_safe(). The current implementation can trigger reading deallocated memory.

Quoted 16 lines
drivers/infiniband/ulp/srp/ib_srp.c +15 -0
+                               list_for_each_entry_safe(target, tmp_target,
+                                                        &host->target_list, list) {
+                                       unsigned long flags;
+
+                                       spin_lock_irqsave(target->scsi_host->host_lock,
+                                                         flags);
+                                       if (!target->qp_in_error &&
+                                           target->state == SRP_TARGET_LIVE)
+                                               srp_qp_err_add_timer(target,
+                                                                    srp_dev_loss_tmo);
+                                       spin_unlock_irqrestore(target->scsi_host->host_lock,
+                                                              flags);
+                               }
+                               spin_unlock(&host->target_lock);
+                       }

drivers/infiniband/ulp/srp/ib_srp.c line 2167

Quoted 10 lines
drivers/infiniband/ulp/srp/ib_srp.c +10 -0
+               }
+               break;
+       case IB_EVENT_PORT_ACTIVE:
+       case IB_EVENT_LID_CHANGE:
+       case IB_EVENT_PKEY_CHANGE:
+       case IB_EVENT_SM_CHANGE:
+               list_for_each_entry_safe(host, tmp_host, &srp_dev->dev_list,
+                                        list) {
+                       if (event->element.port_num == host->port) {
+                               spin_lock(&host->target_lock);
Quoted 4 lines

A remark similar to the above: this code isn't safe against concurrent calls of srp_remove_one().

I did not pay attention to the races created by unloading module (same for patch 3/4). I'll rework patches and address these races.

thanks,
-vu
--
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

author
#5 lore raw
In reply to a message not in the archive
Quoted 21 lines · Bart Van Assche
Quoted 21 lines
(quoted diff) +19 -0
+static void srp_event_handler(struct ib_event_handler *handler,
+                             struct ib_event *event)
+{
+       struct srp_device *srp_dev =
+           ib_get_client_data(event->device, &srp_client);
+       struct srp_host *host, *tmp_host;
+       struct srp_target_port *target, *tmp_target;
+
+       if (!srp_dev || srp_dev->dev != event->device)
+               return;
+
+       printk(KERN_WARNING PFX "ASYNC event= %d on device= %s\n",
+               event->event, srp_dev->dev->name);
+
+       switch (event->event) {
+       case IB_EVENT_PORT_ERR:
+               list_for_each_entry_safe(host, tmp_host,
+                                        &srp_dev->dev_list, list) {
+                       if (event->element.port_num == host->port) {
Quoted 10 lines
Quoted 2 lines

+ spin_lock(&host->target_lock);

Can srp_remove_work() be executed concurrently with srp_event_handler() ? In that case the above code isn't safe and the spin_lock(&host->target_lock) should be moved to just before list_for_each_entry_safe(). The current implementation can trigger reading deallocated memory.

Here is the updated patch to address the race

Handling async local port events: port error, active, lid change... Upon local port err, it will set up timer device_loss_timeout seconds to reconnect. If local port active and there is timer, it will delete the timer

 drivers/infiniband/ulp/srp/ib_srp.c |   85 +++++++++++++++++++++++++++++++++++
 drivers/infiniband/ulp/srp/ib_srp.h |    2 +
 2 files changed, 87 insertions(+), 0 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +85 -0
index 12404d5..88dbc17 100644
@@ -2058,6 +2058,81 @@ free_host:
 	return NULL;
 }
 
+static void srp_event_handler(struct ib_event_handler *handler,
+			      struct ib_event *event)
+{
+	struct srp_device *srp_dev =
+	    ib_get_client_data(event->device, &srp_client);
+	struct srp_host *host, *tmp_host;
+	struct srp_target_port *target, *tmp_target;
+
+	if (!srp_dev || srp_dev->dev != event->device)
+		return;
+
+	printk(KERN_WARNING PFX "ASYNC event= %d on device= %s\n",
+		event->event, srp_dev->dev->name);
+
+	switch (event->event) {
+	case IB_EVENT_PORT_ERR:
+		spin_lock(&srp_dev->dev_lock);
+		list_for_each_entry_safe(host, tmp_host,
+					 &srp_dev->dev_list, list) {
+			if (event->element.port_num == host->port) {
+				spin_lock(&host->target_lock);
+				list_for_each_entry_safe(target, tmp_target,
+							 &host->target_list, list) {
+					unsigned long flags;
+
+					spin_lock_irqsave(target->scsi_host->host_lock,
+							  flags);
+					if (!target->qp_in_error &&
+					    target->state == SRP_TARGET_LIVE)
+						srp_qp_err_add_timer(target,
+								     target->device_loss_timeout);
+					spin_unlock_irqrestore(target->scsi_host->host_lock,
+							       flags);
+				}
+				spin_unlock(&host->target_lock);
+			}
+		}
+		spin_unlock(&srp_dev->dev_lock);
+		break;
+	case IB_EVENT_PORT_ACTIVE:
+	case IB_EVENT_LID_CHANGE:
+	case IB_EVENT_PKEY_CHANGE:
+	case IB_EVENT_SM_CHANGE:
+		spin_lock(&srp_dev->dev_lock);
+		list_for_each_entry_safe(host, tmp_host, &srp_dev->dev_list,
+					 list) {
+			if (event->element.port_num == host->port) {
+				spin_lock(&host->target_lock);
+				list_for_each_entry_safe(target, tmp_target,
+							 &host->target_list, list) {
+					unsigned long flags;
+
+					spin_lock_irqsave(target->scsi_host->host_lock,
+							  flags);
+					if (timer_pending(&target->qp_err_timer)
+					    && !target->qp_in_error) {
+						shost_printk(KERN_WARNING PFX,
+							     target->scsi_host,
+							     "delete qp_in_err timer\n");
+						del_timer(&target->qp_err_timer);
+					}
+					spin_unlock_irqrestore(target->scsi_host->host_lock,
+							       flags);
+				}
+				spin_unlock(&host->target_lock);
+			}
+		}
+		spin_unlock(&srp_dev->dev_lock);
+		break;
+	default:
+		break;
+	}
+
+}
+
 static void srp_add_one(struct ib_device *device)
 {
 	struct srp_device *srp_dev;
@@ -2090,6 +2165,7 @@ static void srp_add_one(struct ib_device *device)
 	srp_dev->fmr_page_mask  = ~((u64) srp_dev->fmr_page_size - 1);
 
 	INIT_LIST_HEAD(&srp_dev->dev_list);
+	spin_lock_init(&srp_dev->dev_lock);
 
 	srp_dev->dev = device;
 	srp_dev->pd  = ib_alloc_pd(device);
@@ -2103,6 +2179,11 @@ static void srp_add_one(struct ib_device *device)
 	if (IS_ERR(srp_dev->mr))
 		goto err_pd;
 
+	INIT_IB_EVENT_HANDLER(&srp_dev->event_handler, srp_dev->dev,
+			      srp_event_handler);
+	if (ib_register_event_handler(&srp_dev->event_handler))
+		goto err_pd;
+
 	memset(&fmr_param, 0, sizeof fmr_param);
 	fmr_param.pool_size	    = SRP_FMR_POOL_SIZE;
 	fmr_param.dirty_watermark   = SRP_FMR_DIRTY_SIZE;
@@ -2154,6 +2235,9 @@ static void srp_remove_one(struct ib_device *device)
 
 	srp_dev = ib_get_client_data(device, &srp_client);
 
+	ib_unregister_event_handler(&srp_dev->event_handler);
+
+	spin_lock(&srp_dev->dev_lock);
 	list_for_each_entry_safe(host, tmp_host, &srp_dev->dev_list, list) {
 		device_unregister(&host->dev);
 		/*
@@ -2193,6 +2277,7 @@ static void srp_remove_one(struct ib_device *device)
 
 		kfree(host);
 	}
+	spin_unlock(&srp_dev->dev_lock);
 
 	if (srp_dev->fmr_pool)
 		ib_destroy_fmr_pool(srp_dev->fmr_pool);
drivers/infiniband/ulp/srp/ib_srp.h +2 -0
index daa4bf7..74d1f09 100644
@@ -86,8 +86,10 @@ enum srp_request_type {
 struct srp_device {
 	struct list_head	dev_list;
 	struct ib_device       *dev;
+	spinlock_t		dev_lock;
 	struct ib_pd	       *pd;
 	struct ib_mr	       *mr;
+	struct ib_event_handler event_handler;
 	struct ib_fmr_pool     *fmr_pool;
 	int			fmr_page_shift;
 	int			fmr_page_size;