linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: akpm@linux-foundation.org, grundler@parisc-linux.org,
	lethal@linux-sh.org, linux-kernel@vger.kernel.org,
	linux-arch@vger.kernel.org
Subject: Re: [PATCH -mm 1/2] scsi: remove dma_is_consistent usage in 53c700
Date: Sun, 27 Jun 2010 10:08:48 -0500	[thread overview]
Message-ID: <1277651328.4366.7.camel@mulgrave.site> (raw)
In-Reply-To: <1277633423-5700-1-git-send-email-fujita.tomonori@lab.ntt.co.jp>

On Sun, 2010-06-27 at 19:10 +0900, FUJITA Tomonori wrote:
> 53c700 is the only user of dma_is_consistent():
> 
> BUG_ON(!dma_is_consistent(hostdata->dev, pScript) && L1_CACHE_BYTES < dma_get_cache_alignment());
> 
> The above code tries to see if the system can allocate coherent memory
> or not. It's for some old systems that can't allocate coherent memory
> at all (e.g some parisc systems).

Actually, that's not the right explanation.  The BUG_ON is because of an
efficiency in the driver ... it's nothing to do with the architecture.

The driver uses a set of mailboxes, but for efficiency's sake, it packs
them into a single coherent area and separates the different usages by a
L1 cache stride).  On architectures capable of manufacturing coherent
memory, this is a nice speed up in the DMA infrastructure.  However, for
incoherent architectures, it's fatal if the dma coherence stride is
greater than the L1 cache size, because now we'll get data corruption
due to cacheline interference.  That's what the BUG_ON is checking for.

> I think that we can safely remove the above usage:
> 
> - such old systems haven't triger the above checking for long.
> 
> - the above condition is important for systems that can't allocate
>   coherent memory if these systems do DMA. So probably it would be
>   better to have such checking in arch's DMA initialization code
>   instead of a driver.

Well, we can't check in the architecture because it's a driver specific
thing ... I suppose making it a rule that dma_get_cache_alignment()
*must* be <= L1_CACHE_BYTES fixes it ... we seem to have no architecture
violating that, so just add it to the documentation, and the check can
go.

James

  parent reply	other threads:[~2010-06-27 15:08 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-27 10:10 [PATCH -mm 1/2] scsi: remove dma_is_consistent usage in 53c700 FUJITA Tomonori
2010-06-27 10:10 ` FUJITA Tomonori
2010-06-27 10:10 ` [PATCH -mm 2/2] remove dma_is_consistent() API FUJITA Tomonori
2010-06-27 15:08 ` James Bottomley [this message]
2010-06-28  3:37   ` [PATCH -mm 1/2] scsi: remove dma_is_consistent usage in 53c700 FUJITA Tomonori
2010-06-28 14:55     ` James Bottomley
2010-06-28 21:27       ` Paul Mundt
2010-06-28 21:27         ` Paul Mundt
2010-06-29  6:04       ` FUJITA Tomonori
2010-06-29 13:37         ` James Bottomley
2010-06-30  2:39           ` FUJITA Tomonori

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=1277651328.4366.7.camel@mulgrave.site \
    --to=james.bottomley@hansenpartnership.com \
    --cc=akpm@linux-foundation.org \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=grundler@parisc-linux.org \
    --cc=lethal@linux-sh.org \
    --cc=linux-arch@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).