All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jgarzik@pobox.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: SCSI Mailing List <linux-scsi@vger.kernel.org>
Subject: Re: [RFC][PATCH] allow sleep inside EH hooks
Date: Fri, 27 May 2005 04:36:27 -0400	[thread overview]
Message-ID: <4296DC0B.9060802@pobox.com> (raw)
In-Reply-To: <20050527075924.GA28608@infradead.org>

Christoph Hellwig wrote:
> On Fri, May 27, 2005 at 03:48:40AM -0400, Jeff Garzik wrote:
> 
>>Christoph Hellwig wrote:
>>
>>>On Fri, May 27, 2005 at 12:32:07AM -0400, Jeff Garzik wrote:
>>>
>>>
>>>>SCSI EH processing already serializes things during EH, so this spinlock 
>>>>isn't really needed.
>>>>
>>>>Removing the spinlock outright would break drivers that surround logic 
>>>>with spin_unlock_irq()..spin_lock_irq(), so I introduced ->unlocked_eh 
>>>>option.
>>>
>>>
>>>Linus has vetoed such conditional locking in the past.  However if you do
>>>it don't make it EH specific but introduce a ->concurrent flag that 
>>>disables
>>>taking host_lock for ->queuecommand aswell.
>>
>>Such a 'concurrent' flag violates Linus credo "do what you must, and no 
>>more."  It's also silly and much too invasive.
>>
>>Removing the locking from the EH routines (only), and fixing up all 
>>necessary drivers, is much more appealing.
> 
> 
> No, hav ing the host_lock only held for ->queuecommand which doesn't
> need that locking doesn't make any sense.  An API like the current one

It makes a lot of sense:  LLDs are written with the assumption that 
paths called from ->queuecommand will not be interrupted by their own 
interrupt handler, whereas error handling paths are typically written 
with precisely the -opposite- assumption.

Removing spin_lock_irq() from queuecommand in SCSI EH causes problems, 
and solves nothing.

	Jeff




  reply	other threads:[~2005-05-27  8:36 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-05-27  4:32 [RFC][PATCH] allow sleep inside EH hooks Jeff Garzik
2005-05-27  6:40 ` Arjan van de Ven
2005-05-27  7:11 ` Christoph Hellwig
2005-05-27  7:48   ` Jeff Garzik
2005-05-27  7:59     ` Christoph Hellwig
2005-05-27  8:36       ` Jeff Garzik [this message]
2005-05-27 16:43         ` Luben Tuikov
2005-05-27 16:49           ` Jeff Garzik
2005-05-27 17:04             ` James Bottomley
2005-05-27 17:22               ` Christoph Hellwig
2005-05-27 17:23                 ` Jeff Garzik
2005-05-27 17:16             ` Christoph Hellwig

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=4296DC0B.9060802@pobox.com \
    --to=jgarzik@pobox.com \
    --cc=hch@infradead.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 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.