Re: [PATCH 3/3] Make blk_cleanup_queue() wait until request_fn finished

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/01/12 19:41, Dan Williams wrote:
On Thu, Sep 27, 2012 at 9:39 AM, Bart Van Assche <bvanassche@xxxxxxx> wrote:
>> [ ... ]
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 593fc71..03571a3 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1517,10 +1517,6 @@ static void scsi_request_fn(struct request_queue *q)
         struct scsi_cmnd *cmd;
         struct request *req;

-       if(!get_device(&sdev->sdev_gendev))
-               /* We must be tearing the block queue down already */
-               return;
-
         /*
          * To start with, we keep looping until the queue is empty, or until
          * the host is no longer able to accept any more requests.
@@ -1629,11 +1625,7 @@ out_delay:
         if (sdev->device_busy == 0)
                 blk_delay_queue(q, SCSI_QUEUE_DELAY);
  out:
-       /* must be careful here...if we trigger the ->remove() function
-        * we cannot be holding the q lock */
-       spin_unlock_irq(q->queue_lock);
-       put_device(&sdev->sdev_gendev);
-       spin_lock_irq(q->queue_lock);
+       ;

Any reason to keep this "out:" label now that it has no effect?

Some people prefer a single-exit style for kernel code since that style makes it easy to add cleanup code for resources allocated inside the function itself. I don't have a strong opinion about this though.

Note: after I posted this patch series I noticed that patch 2/3 leaves a (small) race window. I'm currently testing this follow-up patch:

diff --git a/block/blk-core.c b/block/blk-core.c
index f0efe32..3991f8e 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -425,6 +425,9 @@ void blk_drain_queue(struct request_queue *q, bool drain_all)
                        }
                }

+               if (!drain && blk_queue_dying(q))
+                       queue_flag_set(QUEUE_FLAG_DEAD, q);
+
                spin_unlock_irq(q->queue_lock);

                if (!drain)
@@ -525,10 +528,6 @@ void blk_cleanup_queue(struct request_queue *q)
        /* drain all requests queued before DEAD marking */
        blk_drain_queue(q, true);

-       spin_lock_irq(lock);
-       queue_flag_set(QUEUE_FLAG_DEAD, q);
-       spin_unlock_irq(lock);
-
        /* @q won't process any more request, flush async actions */
        del_timer_sync(&q->backing_dev_info.laptop_mode_wb_timer);
        blk_sync_queue(q);
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux