public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: James Bottomley <James.Bottomley@SteelEye.com>
Cc: Jens Axboe <axboe@suse.de>, Christoph Hellwig <hch@infradead.org>,
	SCSI Mailing List <linux-scsi@vger.kernel.org>,
	Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH scsi-misc-2.6 03/05] scsi: make scsi_queue_insert() use blk_requeue_request()
Date: Thu, 21 Apr 2005 09:20:32 +0900	[thread overview]
Message-ID: <4266F1D0.2060003@gmail.com> (raw)
In-Reply-To: <1114039446.5933.17.camel@mulgrave>


  Hello, James.

James Bottomley wrote:
> On Wed, 2005-04-20 at 08:15 +0900, Tejun Heo wrote:
> 
>>-	 * Insert this command at the head of the queue for it's device.
>>-	 * It will go before all other commands that are already in the queue.
>>-	 *
>>-	 * NOTE: there is magic here about the way the queue is plugged if
>>-	 * we have no outstanding commands.
>>-	 * 
>>-	 * Although this *doesn't* plug the queue, it does call the request
>>-	 * function.  The SCSI request function detects the blocked condition
>>-	 * and plugs the queue appropriately.
> 
> 
> This comment still looks appropriate to me ... why do you want to remove
> it?
> 

  Well, the thing is that we don't really care what exactly happens to 
the queue or how the queue is plugged or not.  All we need to do are to 
requeue the request and kick the queue in the ass.  Hmmm, maybe I should 
keep the comment about how the request will be put at the head of the 
queue, but the second part about plugging doesn't really belong here, I 
think.

> 
>>+	 * Requeue the command.
>> 	 */
>>-	blk_insert_request(device->request_queue, cmd->request, 1, cmd, 1);
>>+	spin_lock_irqsave(q->queue_lock, flags);
>>+	blk_requeue_request(q, cmd->request);
>>+	spin_unlock_irqrestore(q->queue_lock, flags);
>>+
>>+	scsi_run_queue(q);
> 
> 
> Really, wouldn't it be much more efficient simply to call blk_run_queue
> ()? since the blocked flags were set above, that's pretty much what
> scsi_run_queue() collapses to.
> 

  Yes, that will be more efficient but I don't think it would make any 
noticeable difference.  IMO, universally using scsi_run_queue() to kick 
scsi request queues is better than mixing blk_run_queue() and 
scsi_run_queue() for probably unnoticeable optimization.  If we start to 
mix'em, we need to rationalize why specific one is chosen in specific 
places and that's just unnecessary.

  Thanks.

-- 
tejun


  reply	other threads:[~2005-04-21  0:20 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-04-19 23:15 [PATCH scsi-misc-2.6 00/05] scsi: change REQ_SPECIAL/REQ_SOFTBARRIER usages Tejun Heo
2005-04-19 23:15 ` [PATCH scsi-misc-2.6 01/05] scsi: make blk layer set REQ_SOFTBARRIER when a request is dispatched Tejun Heo
2005-04-20  6:30   ` Jens Axboe
2005-04-20  6:44     ` Tejun Heo
2005-04-20  7:40     ` Tejun Heo
2005-04-20  7:58       ` Nick Piggin
2005-04-20  8:37         ` Tejun Heo
2005-04-20  8:38           ` Jens Axboe
2005-04-20  9:04             ` Nick Piggin
2005-04-20  9:14               ` Jens Axboe
2005-04-20  9:24                 ` Nick Piggin
2005-04-20  9:44                   ` Jens Axboe
2005-04-20 22:58                     ` Tejun Heo
2005-04-19 23:15 ` [PATCH scsi-misc-2.6 02/05] scsi: remove REQ_SPECIAL in scsi_init_io() Tejun Heo
2005-04-19 23:15 ` [PATCH scsi-misc-2.6 03/05] scsi: make scsi_queue_insert() use blk_requeue_request() Tejun Heo
2005-04-20 23:24   ` James Bottomley
2005-04-21  0:20     ` Tejun Heo [this message]
2005-04-21  2:16       ` James Bottomley
2005-04-21  2:29         ` Tejun Heo
2005-04-21  2:43         ` Tejun Heo
2005-04-21  6:10     ` Jens Axboe
2005-04-21 12:45       ` James Bottomley
2005-04-22 11:37         ` Jens Axboe
2005-04-19 23:15 ` [PATCH scsi-misc-2.6 04/05] scsi: make scsi_requeue_request() " Tejun Heo
2005-04-19 23:16 ` [PATCH scsi-misc-2.6 05/05] scsi: remove requeue feature from blk_insert_request() Tejun Heo

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=4266F1D0.2060003@gmail.com \
    --to=htejun@gmail.com \
    --cc=James.Bottomley@SteelEye.com \
    --cc=axboe@suse.de \
    --cc=hch@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox