linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Robert Hancock <hancockr@shaw.ca>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Alexander <aledin@mail.ru>,
	linux-kernel@vger.kernel.org, ide <linux-ide@vger.kernel.org>,
	Jeff Garzik <jeff@garzik.org>, Tejun Heo <htejun@gmail.com>
Subject: Re: PROBLEM REMAINS: [sata_nv ADMA breaks ATAPI] Crash on	accessing DVD-RAM
Date: Sat, 12 Jan 2008 17:04:37 -0600	[thread overview]
Message-ID: <47894785.2050508@shaw.ca> (raw)
In-Reply-To: <1200170117.3656.66.camel@localhost.localdomain>

James Bottomley wrote:
>>> With mem<=4098M or sata_nv.adma=0 it still mounts and works ok.
>> As I wrote, it would appear that somehow the blk_queue_bounce_limit 
>> setting that the driver has made is not being respected and the block 
>> layer is still trying to feed it addresses over 4GB. Any ideas anyone?
> 
> Actually, I'd be very sceptical that the blk_queue_bounce_limit isn't
> working as advertised; there'd be a large number of failures if it were
> not.
> 
> However, the "as advertised" part doesn't seem to be generally well
> understood.  The point being that block commands are only bounced if
> they come from the filesystem or the user, not if they're generated
> directly inside the kernel.  Since the fault occurs before mount, it's
> suggestive of the latter.
> 
> A long time ago, GFP_KERNEL allocations meant that the memory allocated
> was physically under 4GB.  Then x86_64 (and before it ia64) wanted to
> break this.  So they introduced a new flag:  GFP_DMA32 that behaved like
> the old GFP_KERNEL and then changed GFP_KERNEL on their architectures to
> return memory from anywhere.  I'd strongly suggest some piece of kernel
> memory was allocated for a transfer buffer without GFP_DMA32 and then
> passed in to the driver.  Unfortunately, that could be anywhere inside
> cdrom or sr.  Knowing what the actual command is might help ... some of
> the distinctive MMC media ones only have a single source.

Just to give some background on what sata_nv is trying to do:

There are two sets of static DMA memory allocations that sata_nv does, 
the legacy ATA PRD and padding buffer which need to be in the lower 4GB, 
and the ADMA APRD and CPB areas which can be anywhere in 64-bit memory. 
With this patch, this is done by setting a 32-bit DMA mask before 
allocating the legacy areas and setting a 64-bit DMA mask before 
allocating the ADMA areas. Previously the driver just set a 64-bit mask 
before the legacy PRD got allocated so it could end up above 4GB, in 
which case legacy DMA couldn't possibly work. That part of the problem 
appears to be successfully fixed by the patch in question.

There's a further problem with runtime DMA mapping, however. Normally 
when ADMA is enabled the controller can reach anywhere in 64-bit memory. 
However, if an ATAPI device is connected, since ADMA doesn't work with 
ATAPI commands we have to switch it off on that port and use legacy DMA, 
which is limited to 32-bit. This is where the blk_queue_bounce_limit 
call comes in, it's trying to make the block layer bounce requests above 
4GB when legacy DMA is in use.

I don't think the problem is that there's some buffer which is getting 
allocated above 4GB and never bounced, since the problem goes away if 
ADMA is disabled entirely and the DMA mask remains 32-bit always. My 
guess is something is basing its decision on whether to bounce or not on 
the device DMA mask. That can't possibly work properly for sata_nv since 
the same PCI device has 2 ports, one of which can be in ADMA mode and 
64-bit capable and the other can be in legacy mode and only 32-bit capable.

Tejun, I believe you had a patch that was printing warnings when libata 
tried to program a legacy PRD with an address over 4GB. Could we change 
that to WARN_ON and get someone experiencing this to try it and
see what the stack trace points to?

  reply	other threads:[~2008-01-12 23:05 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <fa.krOVfyFmB2aqGjJIoHCLmQxecWc@ifi.uio.no>
2008-01-11  5:46 ` PROBLEM REMAINS: [sata_nv ADMA breaks ATAPI] Crash on accessing DVD-RAM Robert Hancock
2008-01-12  8:25   ` Alexander
2008-01-12 19:25     ` Robert Hancock
2008-01-12 20:35       ` James Bottomley
2008-01-12 23:04         ` Robert Hancock [this message]
2008-01-12 23:27           ` James Bottomley
2008-01-13  1:38             ` Robert Hancock
2008-01-13  3:12               ` James Bottomley
2008-01-13 13:33               ` Alan Cox
2008-01-13 15:38                 ` James Bottomley
2008-01-13 16:29                   ` Alan Cox
2008-01-13 16:51                     ` James Bottomley
2008-01-15  1:41                       ` Robert Hancock
2008-01-15  2:23                         ` James Bottomley
2008-02-01  0:09 ` Robert Hancock
2008-02-01  9:59   ` Alexander
2008-02-04  8:29   ` Alexander

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=47894785.2050508@shaw.ca \
    --to=hancockr@shaw.ca \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=aledin@mail.ru \
    --cc=htejun@gmail.com \
    --cc=jeff@garzik.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).