[openib-general] SRP: [PATCH] Releasing the scsi_host when unloading

[Roland Dreier]

 > +			/*
 > +			 * We need 2 scsi_host_put becuase there are two get:
 > +			 *  in scsi_host_alloc and in scsi_add_host
 > +			 */
 > +			scsi_host_put(target->scsi_host);
 >  			scsi_host_put(target->scsi_host);

Hmm, this doesn't seem right to me.  If I try this, then I get a crash
because the scsi_host is already gone after the first put.  I verified
that the reference count is 1 before these puts, and with the
unmodified module I don't see anything left in /sys/class/scsi_host
after unloading the module.

What kernel are you seeing problems with?  I'm testing with an
up-to-date git kernel, although I doubt it makes a difference (did
SCSI reference counting change recently??).

I do think there are some extra scsi_host_put() calls in
srp_remove_work() -- I think the double scsi_host_put() dates back to
a version (which I may never even have checked in) where there was a
scsi_host_get() to avoid the scsi_host going away between the
schedule_work() and srp_remove_work() actually running.

So the patch below seems correct to me.

What do you think?

--- linux-kernel/infiniband/ulp/srp/ib_srp.c	(revision 7245)
+++ linux-kernel/infiniband/ulp/srp/ib_srp.c	(working copy)
@@ -353,7 +356,6 @@ static void srp_remove_work(void *target
 	spin_lock_irq(target->scsi_host->host_lock);
 	if (target->state != SRP_TARGET_DEAD) {
 		spin_unlock_irq(target->scsi_host->host_lock);
-		scsi_host_put(target->scsi_host);
 		return;
 	}
 	target->state = SRP_TARGET_REMOVED;
@@ -367,8 +369,6 @@ static void srp_remove_work(void *target
 	ib_destroy_cm_id(target->cm_id);
 	srp_free_target_ib(target);
 	scsi_host_put(target->scsi_host);
-	/* And another put to really free the target port... */
-	scsi_host_put(target->scsi_host);
 }
 
 static int srp_connect_target(struct srp_target_port *target)