All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Haverkamp <markh@osdl.org>
To: Jens Axboe <axboe@suse.de>
Cc: James Bottomley <James.Bottomley@steeleye.com>,
	Nick Piggin <piggin@cyberone.com.au>,
	Andrew Morton <akpm@osdl.org>, Cliff White <cliffw@osdl.org>,
	linux-scsi <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH] as i/o hang with aacraid driver 2.6.0-test1
Date: 16 Jul 2003 15:45:17 -0700	[thread overview]
Message-ID: <1058395517.2890.1.camel@markh1.pdx.osdl.net> (raw)
In-Reply-To: <20030716132036.GB833@suse.de>

On Wed, 2003-07-16 at 06:20, Jens Axboe wrote:
> On Wed, Jul 16 2003, James Bottomley wrote:
> > > SCSI does it right, don't worry. It's a block layer problem that we need
> > > to find out how to correctly make sure that we deal it. Basically
> > > elv_next_request() does an implicit elv_started_request(), at least that
> > > is what AS assumes. So request re-adds should notify the scheduler of
> > > that fact.
> > 
> > What about exporting a more generic requeue function (perhaps one that
> > would take care of ending the tag)?
> > 
> > Our current requeue code looks like
> > 
> > 	if (blk_rq_tagged(req))
> > 		blk_queue_end_tag(q, req);
> > 	__elv_add_request(q, req, 0, 0);
> > 
> > which looks slightly fragile given that we have to remember to end the
> > tag and then call a __ function (which are usually private).
> 
> Not usually private, but lockless.
> 
> > If we called an explicit request requeue hook, schedulers that need to
> > adjust for requeueing would have a single place they could plug into?
> 
> blk_insert_request() is close, but it does a little more (it's useful
> another place in the scsi queueing, though). How about something like
> this? Then I can add the 'back on queue but not completed' stuff after
> that.
> 
> ===== drivers/block/ll_rw_blk.c 1.192 vs edited =====
> --- 1.192/drivers/block/ll_rw_blk.c	Sun Jul 13 14:15:43 2003
> +++ edited/drivers/block/ll_rw_blk.c	Wed Jul 16 15:17:23 2003
> @@ -1494,6 +1494,23 @@
>  
>  	return rq;
>  }
> +/**
> + * blk_requeue_request - put a request back on queue
> + * @q:		request queue where request should be inserted
> + * @rq:		request to be inserted
> + *
> + * Description:
> + *    Drivers often keep queueing requests until the hardware cannot accept
> + *    more, when that condition happens we need to put the request back
> + *    on the queue. Must be called with queue lock held.
> + */
> +void blk_requeue_request(request_queue_t *q, struct request *rq)
> +{
> +	if (blk_rq_tagged(rq))
> +		blk_queue_end_tag(q, rq);
> +
> +	__elv_add_request(q, rq, 0, 0);
> +}
>  
>  /**
>   * blk_insert_request - insert a special request in to a request queue
> ===== drivers/scsi/scsi_lib.c 1.99 vs edited =====
> --- 1.99/drivers/scsi/scsi_lib.c	Mon Jun 30 03:14:44 2003
> +++ edited/drivers/scsi/scsi_lib.c	Wed Jul 16 15:17:52 2003
> @@ -444,22 +444,8 @@
>   */
>  static void scsi_requeue_command(struct request_queue *q, struct scsi_cmnd *cmd)
>  {
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(q->queue_lock, flags);
> -	cmd->request->special = cmd;
> -	if (blk_rq_tagged(cmd->request))
> -		blk_queue_end_tag(q, cmd->request);
> -
> -	/*
> -	 * set REQ_SPECIAL - we have a command
> -	 * clear REQ_DONTPREP - we assume the sg table has been 
> -	 *	nuked so we need to set it up again.
> -	 */
> -	cmd->request->flags |= REQ_SPECIAL;
>  	cmd->request->flags &= ~REQ_DONTPREP;
> -	__elv_add_request(q, cmd->request, 0, 0);
> -	spin_unlock_irqrestore(q->queue_lock, flags);
> +	blk_insert_request(cmd->request, 1, cmd);

blk_insert_request should have the q pointer as the first arg shouldn't
it?

Mark.


>  
>  	scsi_run_queue(q);
>  }
> @@ -1213,9 +1199,7 @@
>  	 * later time.
>  	 */
>  	spin_lock_irq(q->queue_lock);
> -	if (blk_rq_tagged(req))
> -		blk_queue_end_tag(q, req);
> -	__elv_add_request(q, req, 0, 0);
> +	blk_requeue_request(q, req);
>  	sdev->device_busy--;
>  	if(sdev->device_busy == 0)
>  		blk_plug_device(q);
> ===== include/linux/blkdev.h 1.116 vs edited =====
> --- 1.116/include/linux/blkdev.h	Sat Jul  5 08:52:51 2003
> +++ edited/include/linux/blkdev.h	Wed Jul 16 15:18:06 2003
> @@ -491,6 +491,7 @@
>  extern struct request *blk_get_request(request_queue_t *, int, int);
>  extern void blk_put_request(struct request *);
>  extern void blk_insert_request(request_queue_t *, struct request *, int, void *);
> +extern void blk_requeue_request(request_queue_t *, struct request *);
>  extern void blk_plug_device(request_queue_t *);
>  extern int blk_remove_plug(request_queue_t *);
>  extern void blk_recount_segments(request_queue_t *, struct bio *);
-- 
Mark Haverkamp <markh@osdl.org>


  parent reply	other threads:[~2003-07-16 22:30 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-07-15 23:02 [PATCH] as i/o hang with aacraid driver 2.6.0-test1 Mark Haverkamp
2003-07-16  1:40 ` Nick Piggin
2003-07-16  5:53   ` Jens Axboe
2003-07-16 12:41 ` James Bottomley
2003-07-16 12:45   ` Jens Axboe
2003-07-16 12:56     ` James Bottomley
2003-07-16 13:20       ` Jens Axboe
2003-07-16 14:07         ` James Bottomley
2003-07-16 17:04           ` Jens Axboe
2003-07-17  8:57             ` Andrew Morton
2003-07-17  8:59               ` Jens Axboe
2003-07-17  9:56                 ` Nick Piggin
2003-07-17 10:29                   ` Jens Axboe
2003-07-17 10:51                     ` Nick Piggin
2003-07-17 10:56                       ` Jens Axboe
2003-07-17 11:09                         ` Nick Piggin
2003-07-17 11:11                           ` Jens Axboe
2003-07-17 11:28                             ` Nick Piggin
2003-07-17 11:29                               ` Jens Axboe
2003-07-17 14:44                               ` Mark Haverkamp
2003-07-17 15:43                                 ` James Bottomley
2003-07-17 20:46                               ` Mark Haverkamp
     [not found]                                 ` <1058481553 .19508.5.camel@markh1.pdx.osdl.net>
2003-07-17 22:39                                 ` Mark Haverkamp
2003-07-17 23:47                                   ` Daniel McNeil
2003-07-18  0:00                                     ` Andrew Morton
2003-07-18  5:14                                       ` Nick Piggin
2003-07-18  5:25                                         ` Andrew Morton
2003-07-18  5:30                                           ` Nick Piggin
2003-07-18  5:35                                             ` Nick Piggin
2003-07-18 14:16                                             ` James Bottomley
2003-07-18 16:30                                               ` Andrew Morton
2003-07-18 16:41                                                 ` James Bottomley
2003-07-18 17:25                                                   ` Alan Cox
2003-07-31  7:40                                                     ` Nick Piggin
2003-07-18 17:45                                                   ` Andrew Morton
2003-07-18 18:34                                                     ` James Bottomley
2003-07-18 14:00                                           ` James Bottomley
2003-07-18 15:03                                         ` Mark Haverkamp
2003-07-18 16:28                                           ` Mark Haverkamp
2003-07-18 16:56                                             ` James Bottomley
2003-07-18 17:46                                               ` Mark Haverkamp
2003-07-18 20:21                                                 ` James Bottomley
2003-07-18 20:39                                                   ` Christoph Hellwig
2003-07-18 20:45                                                   ` Mark Haverkamp
2003-07-19  8:26                                                   ` Jens Axboe
2003-07-31  7:16                                                   ` Nick Piggin
2003-07-31 14:28                                                     ` James Bottomley
2003-07-31 14:40                                                     ` Mark Haverkamp
2003-07-31 22:48                                                       ` Nick Piggin
2003-07-17 10:57                       ` Jens Axboe
2003-07-17 11:08                         ` Nick Piggin
2003-07-17 11:10                           ` Jens Axboe
2003-07-17 11:21                             ` Nick Piggin
2003-07-17 11:23                               ` Jens Axboe
2003-07-17 11:29                                 ` Nick Piggin
2003-07-16 22:45         ` Mark Haverkamp [this message]
2003-07-16 13:06 ` Alan Cox

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=1058395517.2890.1.camel@markh1.pdx.osdl.net \
    --to=markh@osdl.org \
    --cc=James.Bottomley@steeleye.com \
    --cc=akpm@osdl.org \
    --cc=axboe@suse.de \
    --cc=cliffw@osdl.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=piggin@cyberone.com.au \
    /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.