linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@suse.de>
To: Mike Christie <michaelc@cs.wisc.edu>
Cc: Jeff Garzik <jgarzik@pobox.com>, Tejun Heo <htejun@gmail.com>,
	linux-ide@vger.kernel.org, lkml <linux-kernel@vger.kernel.org>,
	SCSI Mailing List <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH] libata error handling fixes (ATAPI)
Date: Tue, 15 Nov 2005 19:41:31 +0100	[thread overview]
Message-ID: <20051115184131.GJ7787@suse.de> (raw)
In-Reply-To: <437A2814.1060308@cs.wisc.edu>

On Tue, Nov 15 2005, Mike Christie wrote:
> Jens Axboe wrote:
> >On Tue, Nov 15 2005, Jeff Garzik wrote:
> >
> >>>For departure of libata from SCSI, I was thinking more of another more 
> >>>generic block device framework in which libata can live in.  And I 
> >>>thought that it was reasonable to assume that the framework would supply 
> >>>a EH mechanism which supports queue stalling/draining and separate 
> >>>thread.  So, my EH patches tried to make the same environment for libata 
> >>
> >>A big reason why libata uses the SCSI layer is infrastructure like this. 
> >>It would certainly be nice to see timeouts and EH at the block layer. 
> >>The block layer itself already supports queue stalling/draining.
> >
> >
> >I have a pretty simple plan for this:
> >
> >- Add a timer to struct request. It already has a timeout field for
> >  SG_IO originated requests, we could easily utilize this in general.
> >  I'm not sure how the querying of timeout would happen so far, it would
> >  probably require a q->set_rq_timeout() hook to ask the low level
> >  driver to set/return rq->timeout for a given request.
> >
> >- Add a timeout hook to struct request_queue that would get invoked from
> >  the timeout handler. Something along the lines of:
> >
> >        - Timeout on a request happens. Freeze the queue and use
> >          kblockd to take the actual timeout into process context, where
> >          we call the queue ->rq_timeout() hook. Unfreeze/reschedule
> >          queue operations based on what the ->rq_timeout() hook tells
> >          us.
> >
> >That is generic enough to be able to arm the timeout automatically from
> >->elevator_activate_req_fn() and dearm it when it completes or gets
> >deactivated. It should also be possible to implement the SCSI error
> >handling on top of that.
> >
> 
> To disable the timeout would you then have scsi_done call a block layer 
> function to disarm it then follow the current flow where or do you think 
> it would be nice to move the scsi softirq code up to block layer. So 
> scsi_done would call a block layer function that would disarm the timer, 
> add the request to a block layer softirq list (a list like scsi-ml's 
> scsi_done_q), and then in the block layer softirq function it could call 
> a request_queue callout which for scsi-ml's device queue would call 
> scsi_decide_disposition and return if it wanted the request requeued or 
> how many sectors completed or to kick off the eh. I had stated on this 
> for my block layer multipath driver, but can seperate the patches if 
> this would be useful.

Yeah, that was part of my plan as well. I did post such a patch a year
or so ago, in a thread about decreasing ide completion latencies.

> Would ide benefit from running from a softirq and would it be able to 
> use such a thing?

It's generally useful as it allows lock free completion from the irq
path, so that's goodness.

-- 
Jens Axboe


  reply	other threads:[~2005-11-15 18:41 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-11-14 19:57 [PATCH] libata error handling fixes (ATAPI) Jeff Garzik
2005-11-15  7:41 ` Tejun Heo
2005-11-15  9:28   ` Jeff Garzik
2005-11-15 10:03     ` Tejun Heo
2005-11-15 11:02       ` Jeff Garzik
2005-11-15 12:00         ` Jens Axboe
2005-11-15 18:25           ` Mike Christie
2005-11-15 18:41             ` Jens Axboe [this message]
2005-11-16 12:40               ` Jens Axboe
2005-11-16 12:56                 ` Jeff Garzik
2005-11-16 13:13                   ` Jens Axboe
2005-11-16 13:23                     ` Jeff Garzik
2005-11-16 13:31                 ` Jeff Garzik
2005-11-16 13:47                   ` Jens Axboe
2005-11-16 15:04                 ` Bartlomiej Zolnierkiewicz
2005-11-16 15:31                   ` Jens Axboe
2005-11-16 16:06                     ` Bartlomiej Zolnierkiewicz
2005-11-16 17:10                       ` Jens Axboe
2005-11-16 19:11                         ` Bartlomiej Zolnierkiewicz
2005-11-16 19:22                           ` Jens Axboe
2005-11-16 19:45                             ` Jens Axboe
2005-11-16 19:46                             ` Bartlomiej Zolnierkiewicz
2005-11-16 19:55                               ` Bartlomiej Zolnierkiewicz
2005-11-16 20:02                                 ` Jens Axboe
2005-11-16 20:23                                   ` Bartlomiej Zolnierkiewicz
2005-11-16 20:40                                     ` Jens Axboe
2005-11-19 10:55                       ` Christoph Hellwig
2005-11-19 13:27                         ` Jens Axboe
2005-11-15 13:43     ` Tejun Heo
2005-11-15 14:11       ` Jeff Garzik
2005-11-16  3:04         ` Tejun Heo
2005-11-16  3:18           ` Jeff Garzik
2005-11-16  3:48             ` Mark Lord
2005-11-16  3:58               ` Jeff Garzik

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=20051115184131.GJ7787@suse.de \
    --to=axboe@suse.de \
    --cc=htejun@gmail.com \
    --cc=jgarzik@pobox.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=michaelc@cs.wisc.edu \
    /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;
as well as URLs for NNTP newsgroup(s).