[PATCH] scsi: fix duplicate host number

[PATCH] scsi: fix duplicate host number

[Joe Eykholt]

Just once, two fcoe instances got the same host number
from scsi_add_host().

Use atomic_t and atomic_inc_return() to get next host number.
Subtract 1, so that scsi_host still starts with 0.

Signed-off-by: Joe Eykholt <jeykholt@cisco.com>
---
 drivers/scsi/hosts.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)


diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 89d41a4..e9aaba9 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -40,7 +40,7 @@
 #include "scsi_logging.h"
 
 
-static int scsi_host_next_hn;		/* host_no for next new host */
+static atomic_t scsi_host_next_hn;	/* host_no for next new host */
 
 
 static void scsi_host_cls_release(struct device *dev)
@@ -333,7 +333,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 
 	mutex_init(&shost->scan_mutex);
 
-	shost->host_no = scsi_host_next_hn++; /* XXX(hch): still racy */
+	shost->host_no = atomic_inc_return(&scsi_host_next_hn) - 1;
 	shost->dma_channel = 0xff;
 
 	/* These three are default values which can be overridden */

Re: [PATCH] scsi: fix duplicate host number

[Stefan Richter]

Joe Eykholt wrote:
> Subtract 1, so that scsi_host still starts with 0.
...
> -static int scsi_host_next_hn;		/* host_no for next new host */
> +static atomic_t scsi_host_next_hn;	/* host_no for next new host */
>  
>  
>  static void scsi_host_cls_release(struct device *dev)
> @@ -333,7 +333,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
>  
>  	mutex_init(&shost->scan_mutex);
>  
> -	shost->host_no = scsi_host_next_hn++; /* XXX(hch): still racy */
> +	shost->host_no = atomic_inc_return(&scsi_host_next_hn) - 1;

Can also be written

static atomic_t scsi_host_next_hn = ATOMIC_INIT(-1);
...
	shost->host_no = atomic_inc_return(&scsi_host_next_hn);
-- 
Stefan Richter
-=====-==--= -==- =----
http://arcgraph.de/sr/

Re: [PATCH] scsi: fix duplicate host number

[Joe Eykholt]

Stefan Richter wrote:
> Joe Eykholt wrote:
>> Subtract 1, so that scsi_host still starts with 0.
> ...
>> -static int scsi_host_next_hn;        /* host_no for next new host */
>> +static atomic_t scsi_host_next_hn;    /* host_no for next new host */
>>  
>>  
>>  static void scsi_host_cls_release(struct device *dev)
>> @@ -333,7 +333,7 @@ struct Scsi_Host *scsi_host_alloc(struct 
>> scsi_host_template *sht, int privsize)
>>  
>>      mutex_init(&shost->scan_mutex);
>>  
>> -    shost->host_no = scsi_host_next_hn++; /* XXX(hch): still racy */
>> +    shost->host_no = atomic_inc_return(&scsi_host_next_hn) - 1;
> 
> Can also be written
> 
> static atomic_t scsi_host_next_hn = ATOMIC_INIT(-1);
> ...
>     shost->host_no = atomic_inc_return(&scsi_host_next_hn);

Yes, I considered that, but then the variable name and comment wouldn't be correct.

Also, the initializer adds 4 bytes to the initialized data section, and
the code for -1 should increase text size by less than that, so I think it's a wash.
An atomic_fetch_and_add() would be handy.

	Cheers,
	Joe