On Thu, 2005-04-07 at 08:49 +0200, Jens Axboe wrote:
> On Wed, Apr 06 2005, James Bottomley wrote:
> > My proposal is to correct this by moving the data back to the correct
> > object, and make any object using it hold a reference, so this would
> > make the provider of the block request_fn hold a reference to the queue
> > and initialise the queue lock pointer with the lock currently in the
> > queue. Drivers that still use a global lock would be unaffected. This
>
> But this is the current requirement, as long as you use the queue you
> must hold a reference to it.
Exactly! that's why I think this solution must work independently of
subsystem.
> What do you think of the attached, then? Allow NULL lock to be passed
> in, in which case we use the queue private lock (that no one should ever
> ever touch). It looks a little confusing that
> sdev->request_queue->queue_lock now protects some sdev structures, if
> you want we can retain ->sdev_lock but as a pointer to the queue lock
> instead.
Looks good. How about the attached modification? It makes sdev_lock a
pointer that uses the queue lock which we null out when we release it
(not that I don't trust SCSI or anything ;-)
James
===== drivers/block/ll_rw_blk.c 1.287 vs edited =====
--- 1.287/drivers/block/ll_rw_blk.c 2005-03-11 15:32:27 -05:00
+++ edited/drivers/block/ll_rw_blk.c 2005-04-07 09:05:19 -04:00
@@ -1714,6 +1714,15 @@
if (blk_init_free_list(q))
goto out_init;
+ /*
+ * if caller didn't supply a lock, they get per-queue locking with
+ * our embedded lock
+ */
+ if (!lock) {
+ spin_lock_init(&q->__queue_lock);
+ lock = &q->__queue_lock;
+ }
+
q->request_fn = rfn;
q->back_merge_fn = ll_back_merge_fn;
q->front_merge_fn = ll_front_merge_fn;
===== drivers/scsi/scsi_lib.c 1.151 vs edited =====
--- 1.151/drivers/scsi/scsi_lib.c 2005-02-17 14:17:22 -05:00
+++ edited/drivers/scsi/scsi_lib.c 2005-04-07 09:10:31 -04:00
@@ -349,9 +349,9 @@
shost->host_failed))
scsi_eh_wakeup(shost);
spin_unlock(shost->host_lock);
- spin_lock(&sdev->sdev_lock);
+ spin_lock(sdev->sdev_lock);
sdev->device_busy--;
- spin_unlock_irqrestore(&sdev->sdev_lock, flags);
+ spin_unlock_irqrestore(sdev->sdev_lock, flags);
}
/*
@@ -1391,7 +1391,7 @@
struct Scsi_Host *shost = sdev->host;
struct request_queue *q;
- q = blk_init_queue(scsi_request_fn, &sdev->sdev_lock);
+ q = blk_init_queue(scsi_request_fn, NULL);
if (!q)
return NULL;
===== drivers/scsi/scsi_scan.c 1.142 vs edited =====
--- 1.142/drivers/scsi/scsi_scan.c 2005-02-16 13:21:35 -05:00
+++ edited/drivers/scsi/scsi_scan.c 2005-04-07 09:10:01 -04:00
@@ -249,7 +249,6 @@
*/
sdev->borken = 1;
- spin_lock_init(&sdev->sdev_lock);
sdev->request_queue = scsi_alloc_queue(sdev);
if (!sdev->request_queue) {
/* release fn is set up in scsi_sysfs_device_initialise, so
@@ -257,7 +256,8 @@
put_device(&starget->dev);
goto out;
}
-
+ /* Now make the sdev_lock point to the request queue lock */
+ sdev->sdev_lock = q->queue_lock;
sdev->request_queue->queuedata = sdev;
scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
===== drivers/scsi/scsi_sysfs.c 1.69 vs edited =====
--- 1.69/drivers/scsi/scsi_sysfs.c 2005-02-16 20:05:37 -05:00
+++ edited/drivers/scsi/scsi_sysfs.c 2005-04-07 09:12:17 -04:00
@@ -168,6 +168,9 @@
list_del(&sdev->starved_entry);
spin_unlock_irqrestore(sdev->host->host_lock, flags);
+ /* After releasing the queue we may no longer access its lock */
+ BUG_ON(spin_is_locked(sdev->sdev_lock));
+ sdev->sdev_lock = NULL;
if (sdev->request_queue)
scsi_free_queue(sdev->request_queue);
===== include/linux/blkdev.h 1.161 vs edited =====
--- 1.161/include/linux/blkdev.h 2005-03-09 03:03:24 -05:00
+++ edited/include/linux/blkdev.h 2005-04-07 09:05:21 -04:00
@@ -355,8 +355,11 @@
unsigned long queue_flags;
/*
- * protects queue structures from reentrancy
+ * protects queue structures from reentrancy. ->__queue_lock should
+ * _never_ be used directly, it is queue private. always use
+ * ->queue_lock.
*/
+ spinlock_t __queue_lock;
spinlock_t *queue_lock;
/*
===== include/scsi/scsi_device.h 1.32 vs edited =====
--- 1.32/include/scsi/scsi_device.h 2005-02-17 14:15:51 -05:00
+++ edited/include/scsi/scsi_device.h 2005-04-07 09:08:25 -04:00
@@ -44,7 +44,7 @@
struct list_head same_target_siblings; /* just the devices sharing same target id */
volatile unsigned short device_busy; /* commands actually active on low-level */
- spinlock_t sdev_lock; /* also the request queue_lock */
+ spinlock_t *sdev_lock; /* pointer to the request queue_lock */
spinlock_t list_lock;
struct list_head cmd_list; /* queue of in use SCSI Command structures */
struct list_head starved_entry;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
