public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@SteelEye.com>
To: Tejun Heo <htejun@gmail.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 04/04] scsi: remove unnecessary scsi_wait_req_end_io()
Date: Sat, 14 May 2005 12:19:07 -0400	[thread overview]
Message-ID: <1116087547.5049.25.camel@mulgrave> (raw)
In-Reply-To: <20050514154733.GA5557@htj.dyndns.org>

On Sun, 2005-05-15 at 00:47 +0900, Tejun Heo wrote:
>  BLKPREP_KILL is only used to kill illegal (unpreparable, way-off)
> requests.  Actually, for special requests, the only tests performed
> are req->flags and CDB_SIZE tests.  I don't think anyone does/will
> submit that illegal requests via scsi_wait_req().  And if so, it will
> be a bug.

True, but without the code you're removing it will simply hang the
system, which isn't a correct response to a detected bug.  And if I had
a shilling for every time someone's predicated a code change on "oh,
users will never do this" ... I'd be reasonably rich.

This also leads naturally into the next observation:  Checking in the
request function should be done.  However, it makes little sense wasting
resources preparing requests we know are going to be killed, so the
correct thing to do seems to be to abstract the checks and do them in
both prep_fn and request_fn.

James



  reply	other threads:[~2005-05-14 16:19 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-05-14 13:57 [PATCH scsi-misc-2.6 00/04] scsi: scsi_request_fn() reimplementation Tejun Heo
2005-05-14 13:57 ` [PATCH scsi-misc-2.6 01/04] scsi: consolidate error handling out of scsi_init_io() into scsi_prep_fn() Tejun Heo
2005-05-14 13:57 ` [PATCH scsi-misc-2.6 02/04] scsi: move request preps in other places into prep_fn() Tejun Heo
2005-05-14 13:57 ` [PATCH scsi-misc-2.6 03/04] scsi: reimplement scsi_request_fn() Tejun Heo
2005-05-14 13:57 ` [PATCH scsi-misc-2.6 04/04] scsi: remove unnecessary scsi_wait_req_end_io() Tejun Heo
2005-05-14 15:26   ` James Bottomley
2005-05-14 15:47     ` Tejun Heo
2005-05-14 16:19       ` James Bottomley [this message]
2005-05-15  1:15         ` Tejun Heo
2005-05-16 16:07           ` James Bottomley
2005-05-16 16:56             ` Tejun Heo
2005-05-15  1:16         ` Tejun Heo
  -- strict thread matches above, loose matches on Subject: below --
2005-04-12 10:32 [PATCH scsi-misc-2.6 00/04] scsi: scsi_request_fn() reimplementation Tejun Heo
2005-04-12 10:33 ` [PATCH scsi-misc-2.6 04/04] scsi: remove unnecessary scsi_wait_req_end_io() 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=1116087547.5049.25.camel@mulgrave \
    --to=james.bottomley@steeleye.com \
    --cc=axboe@suse.de \
    --cc=hch@infradead.org \
    --cc=htejun@gmail.com \
    --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