linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Robert Hancock <hancockr@shaw.ca>
To: Arjan van de Ven <arjan@infradead.org>
Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ata: fix sleep-while-holding-spinlock in sata_nv
Date: Wed, 21 May 2008 08:31:26 -0600	[thread overview]
Message-ID: <4834323E.1060608@shaw.ca> (raw)
In-Reply-To: <20080520214311.2d2aa8d3@infradead.org>

rjan van de Ven wrote:
>> I'm not certain this is safe to do it quite this way. It would be
>> better to keep that spinlock held so that no operations could be in
>> progress on either port while these operations are happening.
> 
> blk_bounce_limit can sleep. that's just a fact of life ;(

Now it can, for no reason. Under the conditions it was used before, it 
never could.

> 
>> It would be better to fix the regression from 
>> 419c434c35614609fd0c79d335c134bf4b88b30b in block/blk_settings.c that 
>> resulted in the blk_queue_bounce_limit allocation wrongly allocating 
>> emergency ISA pages in the first place as a 32-bit DMA mask does not 
>> need them.
> 
> the condition under which it sleeps might be slightly buggy on your
> exact x86 machine... but that doesn't mean that that is guaranteed to
> be so forever going forward.... it's still a sleeping function.

More than slightly buggy, I think.. It seems like it is going to be 
bouncing block layer accesses to devices with 32-bit DMA masks through 
the 16MB ZONE_DMA. If that's what's actually going on, I'm surprised 
there haven't been more regression reports. The fact that the function 
now sleeps when it didn't before is the least of the problems here..

  reply	other threads:[~2008-05-21 14:31 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <fa.m+kShlzXY/wmFlvZXsE0W7wQIxU@ifi.uio.no>
2008-05-21  2:17 ` [PATCH] ata: fix sleep-while-holding-spinlock in sata_nv Robert Hancock
2008-05-21  4:43   ` Arjan van de Ven
2008-05-21 14:31     ` Robert Hancock [this message]
2008-05-21 15:16       ` Arjan van de Ven
2008-05-20 22:58 Arjan van de Ven

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=4834323E.1060608@shaw.ca \
    --to=hancockr@shaw.ca \
    --cc=arjan@infradead.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@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;
as well as URLs for NNTP newsgroup(s).