All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Smart <James.Smart@Emulex.Com>
To: linux-scsi@vger.kernel.org
Cc: james.bottomley@steeleye.com
Subject: [PATCH] bug fix: SCSI async scan sysfs -EEXIST problem
Date: Mon, 07 May 2007 14:57:47 -0400	[thread overview]
Message-ID: <1178564267.302.15.camel@localhost.localdomain> (raw)

This corrects the kobject_add -EEXIST failure reported in 
http://marc.info/?l=linux-scsi&m=117699334422336&w=2

Basically, there was a collision between async scsi scan's
scsi_sysfs_add_devices() and asynchronous scanning kicked off by the
fc transport's rport code.  Both called scsi_sysfs_add_sdev() for the
same sdev.

The problem was that async scsi scan's call did not hold the scan_mutex
when making the call to scsi_sysfs_add_sdev().  Additionally, looking at
the shost->async_scan flag, which is critical for whether the sdev gets
enumerated or not, many uses were fuzzy. As a bit flag, it should have
been under the scsi_host lock, but via the way it's used within
scsi_scan.c, it should have been under the scan_mutex. It was positioned
within the async scan semaphore, which didn't help anything. Key to its
change, is that is done prior to releasing the scan_mutex after around
the scsi_sysfs_add_sdev() calls.  To avoid all the lock repositioning,
I simply converted it to an atomic. A little overkill, but has the
coherency it needs.

Confirmed by Josef that it corrected his problems.

-- james s


Signed-off-by: James Smart <James.Smart@emulex.com>



diff -upNr a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
--- a/drivers/scsi/scsi_scan.c	2007-04-25 23:08:32.000000000 -0400
+++ b/drivers/scsi/scsi_scan.c	2007-05-04 18:35:58.000000000 -0400
@@ -1081,7 +1081,8 @@ static int scsi_probe_and_add_lun(struct
 		goto out_free_result;
 	}
 
-	res = scsi_add_lun(sdev, result, &bflags, shost->async_scan);
+	res = scsi_add_lun(sdev, result, &bflags,
+				atomic_read(&shost->async_scan));
 	if (res == SCSI_SCAN_LUN_PRESENT) {
 		if (bflags & BLIST_KEY) {
 			sdev->lockable = 0;
@@ -1470,7 +1471,7 @@ struct scsi_device *__scsi_add_device(st
 	if (strncmp(scsi_scan_type, "none", 4) == 0)
 		return ERR_PTR(-ENODEV);
 
-	if (!shost->async_scan)
+	if (!atomic_read(&shost->async_scan))
 		scsi_complete_async_scans();
 
 	starget = scsi_alloc_target(parent, channel, id);
@@ -1590,7 +1591,7 @@ void scsi_scan_target(struct device *par
 	if (strncmp(scsi_scan_type, "none", 4) == 0)
 		return;
 
-	if (!shost->async_scan)
+	if (!atomic_read(&shost->async_scan))
 		scsi_complete_async_scans();
 
 	mutex_lock(&shost->scan_mutex);
@@ -1638,7 +1639,7 @@ int scsi_scan_host_selected(struct Scsi_
 		"%s: <%u:%u:%u>\n",
 		__FUNCTION__, channel, id, lun));
 
-	if (!shost->async_scan)
+	if (!atomic_read(&shost->async_scan))
 		scsi_complete_async_scans();
 
 	if (((channel != SCAN_WILD_CARD) && (channel > shost->max_channel)) ||
@@ -1687,7 +1688,7 @@ static struct async_scan_data *scsi_prep
 	if (strncmp(scsi_scan_type, "sync", 4) == 0)
 		return NULL;
 
-	if (shost->async_scan) {
+	if (atomic_read(&shost->async_scan)) {
 		printk("%s called twice for host %d", __FUNCTION__,
 				shost->host_no);
 		dump_stack();
@@ -1702,8 +1703,9 @@ static struct async_scan_data *scsi_prep
 		goto err;
 	init_completion(&data->prev_finished);
 
+	atomic_set(&shost->async_scan, 1);
+
 	spin_lock(&async_scan_lock);
-	shost->async_scan = 1;
 	if (list_empty(&scanning_hosts))
 		complete(&data->prev_finished);
 	list_add_tail(&data->list, &scanning_hosts);
@@ -1732,7 +1734,7 @@ static void scsi_finish_async_scan(struc
 		return;
 
 	shost = data->shost;
-	if (!shost->async_scan) {
+	if (!atomic_read(&shost->async_scan)) {
 		printk("%s called twice for host %d", __FUNCTION__,
 				shost->host_no);
 		dump_stack();
@@ -1741,10 +1743,12 @@ static void scsi_finish_async_scan(struc
 
 	wait_for_completion(&data->prev_finished);
 
+	mutex_lock(&shost->scan_mutex);
 	scsi_sysfs_add_devices(shost);
+	atomic_set(&shost->async_scan, 0);
+	mutex_unlock(&shost->scan_mutex);
 
 	spin_lock(&async_scan_lock);
-	shost->async_scan = 0;
 	list_del(&data->list);
 	if (!list_empty(&scanning_hosts)) {
 		struct async_scan_data *next = list_entry(scanning_hosts.next,
diff -upNr a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
--- a/include/scsi/scsi_host.h	2007-04-25 23:08:32.000000000 -0400
+++ b/include/scsi/scsi_host.h	2007-05-04 17:48:48.000000000 -0400
@@ -6,6 +6,7 @@
 #include <linux/types.h>
 #include <linux/workqueue.h>
 #include <linux/mutex.h>
+#include <asm/atomic.h>
 
 struct request_queue;
 struct block_device;
@@ -605,7 +606,7 @@ struct Scsi_Host {
 	unsigned tmf_in_progress:1;
 
 	/* Asynchronous scan in progress */
-	unsigned async_scan:1;
+	atomic_t async_scan;
 
 	/*
 	 * Optional work queue to be utilized by the transport





             reply	other threads:[~2007-05-07 18:57 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-07 18:57 James Smart [this message]
2007-05-22 15:48 ` [PATCH] bug fix: SCSI async scan sysfs -EEXIST problem James Bottomley
2007-05-22 21:10   ` James Smart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1178564267.302.15.camel@localhost.localdomain \
    --to=james.smart@emulex.com \
    --cc=james.bottomley@steeleye.com \
    --cc=linux-scsi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.