Re: oops in ibmvstgt

[Brian King <brking@linux.vnet.ibm.com>]

FUJITA Tomonori wrote:
> On Wed, 3 Dec 2008 15:58:57 +0100
> Olaf Hering <olh@suse.de> wrote:
> 
>> change init order to fix crash due to uninitalized shost_data
> 
> I think that calling crq_queue_create leads to the creation of a rport
> so we need to set up everything before that. The current code is racy.
> 
> 
>> No idea if the patch is correct.
> 
> Almost correct, I think. Needs to modify the error handling
> too. Probably, it would be better to call scsi_tgt_alloc_queue before
> crq_queue_create.
> 
> How about this?

Don't we need to do a little more in the error handling like this:

-Brian

=
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH] ibmvstgt: move crq_queue_create at the end of initialization

Calling crq_queue_create could lead to the creation of a rport. We
need to set up everything before creating a rport. This moves
crq_queue_create at the end of initialization to avoid a race.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---

index 2a5b29d..8dcb59e 100644
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---

 drivers/scsi/ibmvscsi/ibmvstgt.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff -puN drivers/scsi/ibmvscsi/ibmvstgt.c~scsi_oops_in_ibmvstgt drivers/scsi/ibmvscsi/ibmvstgt.c
--- linux-2.6/drivers/scsi/ibmvscsi/ibmvstgt.c~scsi_oops_in_ibmvstgt	2008-12-08 13:04:21.000000000 -0600
+++ linux-2.6-bjking1/drivers/scsi/ibmvscsi/ibmvstgt.c	2008-12-08 13:16:32.000000000 -0600
@@ -864,21 +864,23 @@ static int ibmvstgt_probe(struct vio_dev
 
 	INIT_WORK(&vport->crq_work, handle_crq);
 
-	err = crq_queue_create(&vport->crq_queue, target);
+	err = scsi_add_host(shost, target->dev);
 	if (err)
 		goto free_srp_target;
 
-	err = scsi_add_host(shost, target->dev);
+	err = scsi_tgt_alloc_queue(shost);
 	if (err)
-		goto destroy_queue;
+		goto remove_host;
 
-	err = scsi_tgt_alloc_queue(shost);
+	err = crq_queue_create(&vport->crq_queue, target);
 	if (err)
-		goto destroy_queue;
+		goto free_queue;
 
 	return 0;
-destroy_queue:
-	crq_queue_destroy(target);
+free_queue:
+	scsi_tgt_free_queue(shost);
+remove_host:
+	scsi_remove_host(shost);
 free_srp_target:
 	srp_target_free(target);
 put_host:
_