All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vaughan Cao <vaughan.cao@oracle.com>
To: joern@logfs.org
Cc: dgilbert@interlog.com, JBottomley@parallels.com,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
	vaughan.cao@oracle.com
Subject: [PATCH v5 4/4] [SCSI] sg: push file descriptor list locking down to per-device locking
Date: Mon, 22 Jul 2013 12:40:33 +0800	[thread overview]
Message-ID: <1374468033-8947-5-git-send-email-vaughan.cao@oracle.com> (raw)
In-Reply-To: <1374468033-8947-1-git-send-email-vaughan.cao@oracle.com>

Push file descriptor list locking down to per-device locking. Let sg_index_lock
only protect device lookup.
sdp->detached is also set and checked with this lock held.

Changes from v4:
 * Since I use ERR_PTR and friends in sg_add_sfp, this patch should also be
updated to resolve conflict in cherrry-pick.

Signed-off-by: Vaughan Cao <vaughan.cao@oracle.com>
---
 drivers/scsi/sg.c | 61 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index f0e4785..3431d12 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -104,8 +104,7 @@ static int sg_add(struct device *, struct class_interface *);
 static void sg_remove(struct device *, struct class_interface *);
 
 static DEFINE_IDR(sg_index_idr);
-static DEFINE_RWLOCK(sg_index_lock);	/* Also used to lock
-							   file descriptor list for device */
+static DEFINE_RWLOCK(sg_index_lock);
 
 static struct class_interface sg_interface = {
 	.add_dev	= sg_add,
@@ -142,8 +141,7 @@ typedef struct sg_request {	/* SG_MAX_QUEUE requests outstanding per file */
 } Sg_request;
 
 typedef struct sg_fd {		/* holds the state of a file descriptor */
-	/* sfd_siblings is protected by sg_index_lock */
-	struct list_head sfd_siblings;
+	struct list_head sfd_siblings; /* protected by sfd_lock of device */
 	struct sg_device *parentdp;	/* owning device */
 	wait_queue_head_t read_wait;	/* queue read until command done */
 	rwlock_t rq_list_lock;	/* protect access to list in req_arr */
@@ -168,7 +166,7 @@ typedef struct sg_device { /* holds the state of each scsi generic device */
 	struct scsi_device *device;
 	int sg_tablesize;	/* adapter's max scatter-gather table size */
 	u32 index;		/* device index number */
-	/* sfds is protected by sg_index_lock */
+	spinlock_t sfd_lock;	/* protect file descriptor list for device */
 	struct list_head sfds;
 	struct rw_semaphore o_sem;	/* exclude open should hold this rwsem */
 	volatile char detached;	/* 0->attached, 1->detached pending removal */
@@ -226,9 +224,9 @@ static int sfds_list_empty(Sg_device *sdp)
 	unsigned long flags;
 	int ret;
 
-	read_lock_irqsave(&sg_index_lock, flags);
+	spin_lock_irqsave(&sdp->sfd_lock, flags);
 	ret = list_empty(&sdp->sfds);
-	read_unlock_irqrestore(&sg_index_lock, flags);
+	spin_unlock_irqrestore(&sdp->sfd_lock, flags);
 	return ret;
 }
 
@@ -1394,6 +1392,7 @@ static Sg_device *sg_alloc(struct gendisk *disk, struct scsi_device *scsidp)
 	disk->first_minor = k;
 	sdp->disk = disk;
 	sdp->device = scsidp;
+	spin_lock_init(&sdp->sfd_lock);
 	INIT_LIST_HEAD(&sdp->sfds);
 	init_rwsem(&sdp->o_sem);
 	sdp->sg_tablesize = queue_max_segments(q);
@@ -1536,11 +1535,13 @@ static void sg_remove(struct device *cl_dev, struct class_interface *cl_intf)
 
 	/* Need a write lock to set sdp->detached. */
 	write_lock_irqsave(&sg_index_lock, iflags);
+	spin_lock(&sdp->sfd_lock);
 	sdp->detached = 1;
 	list_for_each_entry(sfp, &sdp->sfds, sfd_siblings) {
 		wake_up_interruptible(&sfp->read_wait);
 		kill_fasync(&sfp->async_qp, SIGPOLL, POLL_HUP);
 	}
+	spin_unlock(&sdp->sfd_lock);
 	write_unlock_irqrestore(&sg_index_lock, iflags);
 
 	sysfs_remove_link(&scsidp->sdev_gendev.kobj, "generic");
@@ -2065,13 +2066,13 @@ sg_add_sfp(Sg_device * sdp, int dev)
 	sfp->cmd_q = SG_DEF_COMMAND_Q;
 	sfp->keep_orphan = SG_DEF_KEEP_ORPHAN;
 	sfp->parentdp = sdp;
-	write_lock_irqsave(&sg_index_lock, iflags);
+	spin_lock_irqsave(&sdp->sfd_lock, iflags);
 	if (sdp->detached) {
-		write_unlock_irqrestore(&sg_index_lock, iflags);
+		spin_unlock_irqrestore(&sdp->sfd_lock, iflags);
 		return ERR_PTR(-ENODEV);
 	}
 	list_add_tail(&sfp->sfd_siblings, &sdp->sfds);
-	write_unlock_irqrestore(&sg_index_lock, iflags);
+	spin_unlock_irqrestore(&sdp->sfd_lock, iflags);
 	SCSI_LOG_TIMEOUT(3, printk("sg_add_sfp: sfp=0x%p\n", sfp));
 	if (unlikely(sg_big_buff != def_reserved_size))
 		sg_big_buff = def_reserved_size;
@@ -2121,9 +2122,9 @@ static void sg_remove_sfp(struct kref *kref)
 	struct sg_device *sdp = sfp->parentdp;
 	unsigned long iflags;
 
-	write_lock_irqsave(&sg_index_lock, iflags);
+	spin_lock_irqsave(&sdp->sfd_lock, iflags);
 	list_del(&sfp->sfd_siblings);
-	write_unlock_irqrestore(&sg_index_lock, iflags);
+	spin_unlock_irqrestore(&sdp->sfd_lock, iflags);
 
 	INIT_WORK(&sfp->ew.work, sg_remove_sfp_usercontext);
 	schedule_work(&sfp->ew.work);
@@ -2514,7 +2515,7 @@ static int sg_proc_seq_show_devstrs(struct seq_file *s, void *v)
 	return 0;
 }
 
-/* must be called while holding sg_index_lock */
+/* must be called while holding sg_index_lock and sfd_lock */
 static void sg_proc_debug_helper(struct seq_file *s, Sg_device * sdp)
 {
 	int k, m, new_interface, blen, usg;
@@ -2599,22 +2600,26 @@ static int sg_proc_seq_show_debug(struct seq_file *s, void *v)
 
 	read_lock_irqsave(&sg_index_lock, iflags);
 	sdp = it ? sg_lookup_dev(it->index) : NULL;
-	if (sdp && !list_empty(&sdp->sfds)) {
-		struct scsi_device *scsidp = sdp->device;
+	if (sdp) {
+		spin_lock(&sdp->sfd_lock);
+		if (!list_empty(&sdp->sfds)) {
+			struct scsi_device *scsidp = sdp->device;
 
-		seq_printf(s, " >>> device=%s ", sdp->disk->disk_name);
-		if (sdp->detached)
-			seq_printf(s, "detached pending close ");
-		else
-			seq_printf
-			    (s, "scsi%d chan=%d id=%d lun=%d   em=%d",
-			     scsidp->host->host_no,
-			     scsidp->channel, scsidp->id,
-			     scsidp->lun,
-			     scsidp->host->hostt->emulated);
-		seq_printf(s, " sg_tablesize=%d excl=%d\n",
-			   sdp->sg_tablesize, sdp->exclude);
-		sg_proc_debug_helper(s, sdp);
+			seq_printf(s, " >>> device=%s ", sdp->disk->disk_name);
+			if (sdp->detached)
+				seq_printf(s, "detached pending close ");
+			else
+				seq_printf
+				    (s, "scsi%d chan=%d id=%d lun=%d   em=%d",
+				     scsidp->host->host_no,
+				     scsidp->channel, scsidp->id,
+				     scsidp->lun,
+				     scsidp->host->hostt->emulated);
+			seq_printf(s, " sg_tablesize=%d excl=%d\n",
+				   sdp->sg_tablesize, sdp->exclude);
+			sg_proc_debug_helper(s, sdp);
+		}
+		spin_unlock(&sdp->sfd_lock);
 	}
 	read_unlock_irqrestore(&sg_index_lock, iflags);
 	return 0;
-- 
1.7.11.7


  parent reply	other threads:[~2013-07-22  4:38 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-05  9:18 [PATCH] sg: atomize check and set sdp->exclude in sg_open vaughan
2013-06-05 13:27 ` Jörn Engel
2013-06-05 16:16   ` vaughan
2013-06-05 15:41     ` Jörn Engel
2013-06-06  7:19       ` vaughan
2013-06-06  7:29         ` vaughan
2013-06-06  7:29           ` vaughan
2013-06-17 13:10       ` [PATCH v2 1/1] [SCSI] sg: fix race condition when do exclusive open vaughan
2013-06-26  1:37         ` vaughan
2013-07-05  1:59           ` vaughan
2013-07-05  1:59             ` vaughan
2013-07-05 17:39         ` Jörn Engel
2013-07-05 17:39           ` Jörn Engel
2013-07-06 17:24           ` vaughan
2013-07-07 19:53             ` [PATCH v3 " vaughan
2013-07-15 20:37               ` Jörn Engel
2013-07-17 15:34                 ` [PATCH v4 0/4] [SCSI] sg: fix race condition in sg_open Vaughan Cao
2013-07-17 15:34                   ` [PATCH v4 1/4] [SCSI] sg: use rwsem to solve race during exclusive open Vaughan Cao
2013-07-19 21:19                     ` Jörn Engel
2013-07-19 21:19                       ` Jörn Engel
2013-07-17 15:34                   ` [PATCH v4 2/4] [SCSI] sg: no need sg_open_exclusive_lock Vaughan Cao
2013-07-19 21:19                     ` Jörn Engel
2013-07-17 15:34                   ` [PATCH v4 3/4] [SCSI] sg: checking sdp->detached isn't protected when open Vaughan Cao
2013-07-19 21:24                     ` Jörn Engel
2013-07-22  3:39                       ` [PATCH v5 " Vaughan Cao
     [not found]                       ` <CAMvaAQnFy0WiXHaNtAB1KPLK-7yj1AHh=_Pw4MBm0=_ecpoAoQ@mail.gmail.com>
2013-07-22 16:52                         ` [PATCH v4 " Jörn Engel
2013-07-17 15:34                   ` [PATCH v4 4/4] [SCSI] sg: push file descriptor list locking down to per-device locking Vaughan Cao
2013-07-19 21:26                     ` Jörn Engel
2013-07-22  3:41                       ` [PATCH v5 " Vaughan Cao
2013-07-22  4:40                   ` [PATCH v5 0/4] [SCSI] sg: fix race condition in sg_open Vaughan Cao
2013-07-22  4:40                     ` [PATCH v5 1/4] [SCSI] sg: use rwsem to solve race during exclusive open Vaughan Cao
2013-08-28  4:00                       ` James Bottomley
2013-08-28 10:07                         ` [PATCH v6 0/4][SCSI] sg: fix race condition in sg_open Vaughan Cao
2013-08-28 10:07                           ` [PATCH v6 1/4] sg: use rwsem to solve race during exclusive open Vaughan Cao
2013-08-28 10:26                             ` James Bottomley
2013-08-29  2:00                               ` [PATCH v7 0/4][SCSI] sg: fix race condition in sg_open Vaughan Cao
2013-08-29  2:00                                 ` [PATCH v7 1/4] sg: use rwsem to solve race during exclusive open Vaughan Cao
2013-08-29  2:00                                 ` [PATCH v7 2/4] sg: no need sg_open_exclusive_lock Vaughan Cao
2013-08-29  2:00                                 ` [PATCH v7 3/4] sg: checking sdp->detached isn't protected when open Vaughan Cao
2013-08-29  2:00                                 ` [PATCH v7 4/4] sg: push file descriptor list locking down to per-device locking Vaughan Cao
2013-08-28 10:07                           ` [PATCH v6 2/4] sg: no need sg_open_exclusive_lock Vaughan Cao
2013-08-28 10:07                           ` [PATCH v6 3/4] sg: checking sdp->detached isn't protected when open Vaughan Cao
2013-08-28 10:07                           ` [PATCH v6 4/4] sg: push file descriptor list locking down to per-device locking Vaughan Cao
2013-07-22  4:40                     ` [PATCH v5 2/4] [SCSI] sg: no need sg_open_exclusive_lock Vaughan Cao
2013-07-22  4:40                     ` [PATCH v5 3/4] [SCSI] sg: checking sdp->detached isn't protected when open Vaughan Cao
2013-07-22  4:40                     ` Vaughan Cao [this message]
2013-07-22 17:03                     ` [PATCH v5 0/4] [SCSI] sg: fix race condition in sg_open Jörn Engel
2013-07-22 17:03                       ` Jörn Engel
2013-07-25 15:32                       ` vaughan
2013-07-25 15:32                         ` vaughan
2013-07-25 20:33                         ` Douglas Gilbert
2013-07-25 20:33                           ` Douglas Gilbert
2013-07-31  4:40                           ` vaughan
2013-08-01  5:01                       ` Douglas Gilbert
2013-08-03  5:25                         ` Douglas Gilbert
2013-08-05  2:19                           ` vaughan
2013-08-05 20:52                             ` Douglas Gilbert
2013-08-05 20:52                               ` Douglas Gilbert
2013-08-13  2:46                               ` vaughan
2013-08-13  3:16                                 ` Douglas Gilbert
2013-08-27  8:16                                   ` vaughan
2013-08-27 13:13                                     ` Douglas Gilbert
2013-08-28  1:50                                       ` vaughan
2013-07-15 17:25             ` [PATCH v2 1/1] [SCSI] sg: fix race condition when do exclusive open Jörn Engel

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=1374468033-8947-5-git-send-email-vaughan.cao@oracle.com \
    --to=vaughan.cao@oracle.com \
    --cc=JBottomley@parallels.com \
    --cc=dgilbert@interlog.com \
    --cc=joern@logfs.org \
    --cc=linux-kernel@vger.kernel.org \
    --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.