public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Patrick Mansfield <patmans@us.ibm.com>
Cc: James.Bottomley@steeleye.com, linux-scsi@vger.kernel.org
Subject: Re: [PATCH] fixes and cleanups for the new command allocation code
Date: Tue, 4 Feb 2003 17:51:46 +0100	[thread overview]
Message-ID: <20030204175146.A31515@lst.de> (raw)
In-Reply-To: <20030204081616.A24105@beaverton.ibm.com>; from patmans@us.ibm.com on Tue, Feb 04, 2003 at 08:16:16AM -0800

On Tue, Feb 04, 2003 at 08:16:16AM -0800, Patrick Mansfield wrote:
> We are missing the above checks and functionallity in scsi_get_command.
> 
> The limit previously imposed by current_queue_depth via the number of
> scmnd's allocated is not checked - scsi_get_command should check
> device_busy < new_queue_depth.
> 
> I was trying to fix/hit this - surprisingly, I did not see performance
> problems (i.e. getting tons of QUEUE_FULLs), probably because my request
> queue limits are 128, and the disks are not old.

I wonder whether we really need it or whether the queue limits shouldn't
be enough.  If there's a chance I'd like to avoid having throttewling in
too many places.

> So one of the above needs to (conditionally ... based on gfp_mask?) get
> host/queue_lock, check limits, and conditionally add_wait_queue().

I don't think we need to add the waitqeue.  The scsi midlayer always
calls scsi_get_command with an GFP_ATOMIC argument, so we can't ever
wait, so this would only apply to the gdth drivers that calls it directly.
And even this driver only uses it for administrative commands (i.e. not
in the I/O) and the driver doesn't even compile in 2.5 :)

> What protects pool->users?

I assumed adding/removing hosts was serialized but it isn't.  I'll add
code to fix this in the next drop.


  reply	other threads:[~2003-02-04 16:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-02-04 15:23 [PATCH] fixes and cleanups for the new command allocation code Christoph Hellwig
2003-02-04 16:16 ` Patrick Mansfield
2003-02-04 16:51   ` Christoph Hellwig [this message]
2003-02-04 17:19     ` Patrick Mansfield
2003-02-04 17:57       ` Luben Tuikov
2003-02-04 18:03         ` Christoph Hellwig
2003-02-04 18:08           ` Luben Tuikov
2003-02-04 18:33       ` James Bottomley
2003-02-04 19:29         ` Christoph Hellwig
2003-02-04 23:03           ` James Bottomley
2003-02-05  1:25             ` Patrick Mansfield
2003-02-05  1:53               ` James Bottomley
2003-02-05  5:15                 ` Patrick Mansfield
2003-02-05 15:22                   ` James Bottomley
2003-02-05 15:59                     ` James Bottomley

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=20030204175146.A31515@lst.de \
    --to=hch@lst.de \
    --cc=James.Bottomley@steeleye.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=patmans@us.ibm.com \
    /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