LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Grant Grundler <grundler@parisc-linux.org>
To: Michael Ellerman <michael@ellerman.id.au>
Cc: akepner@sgi.com, Tony Luck <tony.luck@intel.com>,
	Jesse Barnes <jbarnes@virtuousgeek.org>,
	Jes Sorensen <jes@sgi.com>,
	Randy Dunlap <randy.dunlap@oracle.com>,
	Roland Dreier <rdreier@cisco.com>,
	James Bottomley <James.Bottomley@hansenpartnership.com>,
	David Miller <davem@davemloft.net>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Grant Grundler <grundler@parisc-linux.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3 v3] dma: document dma_{un}map_{single|sg}_attrs() interface
Date: Fri, 29 Feb 2008 11:25:04 -0700	[thread overview]
Message-ID: <20080229182504.GA18102@colo.lackof.org> (raw)
In-Reply-To: <dc1166600802281845t1613beedj7a285399a21cd@mail.gmail.com>

On Fri, Feb 29, 2008 at 01:45:46PM +1100, Michael Ellerman wrote:
> On Thu, Feb 28, 2008 at 2:24 PM,  <akepner@sgi.com> wrote:
> >
> >  Document the new dma_{un}map_{single|sg}_attrs() functions.
> >
> >  diff --git a/Documentation/DMA-attributes.txt b/Documentation/DMA-attributes.txt
> >  index e69de29..36baea5 100644
> >  --- a/Documentation/DMA-attributes.txt
> >  +++ b/Documentation/DMA-attributes.txt
> >  @@ -0,0 +1,29 @@
> >  +                       DMA attributes
> >  +                       ==============
> >  +
> >  +This document describes the semantics of the DMA attributes that are
> >  +defined in linux/dma-attrs.h.
> >  +
> >  +
> >  +DMA_ATTR_SYNC_ON_WRITE
> >  +----------------------
> >  +
> >  +DMA_ATTR_SYNC_ON_WRITE is used on the IA64_SGI_SN2 architecture.
> >  +It provides a mechanism for devices to explicitly order their DMA
> >  +writes.
> >  +
> >  +On IA64_SGI_SN2 machines, DMA may be reordered within the NUMA
> >  +interconnect. Allowing reordering improves performance, but in some
> >  +situations it may be necessary to ensure that one DMA write is
> >  +complete before another is visible. For example, if the device does
> >  +a DMA write to indicate that data is available in memory, DMA of the
> >  +"completion indication" can race with DMA of data.
> >  +
> >  +When a memory region is mapped with the DMA_ATTR_SYNC_ON_WRITE attribute,
> >  +a write to that region causes all in-flight DMA to be flushed to memory.
> >  +Any pending DMA will complete and be visible in memory before the write
> >  +to the region with the DMA_ATTR_SYNC_ON_WRITE attribute becomes visible.
> 
> 
> I'm not clear how this is all meant to work. Your intial patch says
> this is an interface to pass "architecture-specific
> attributes" from drivers through to the DMA mapping code, which is
> fair enough - we want to do something similar.
> 
> But it's not clear that DMA_ATTR_SYNC_ON_WRITE is architecture
> specific. If I was a driver writer might assume it works on all
> platforms.

That would be a fair assumption. But it is required to be a NOP on
platforms that don't need the "hint" ("attr" or whatever you want
to call it). Specific architectures (SN2 in this case) will need
to implement something.

> And in fact in patch 3/3 you define it in
> linux/dma-attrs.h, so it's not architecture specific.

Correct. The same driver needs to compile on all architectures.

> What is architecture specific is the semantic. DMA_ATTR_SYNC_ON_WRITE
> is defined entirely in terms of what it does on ia64 hardware, and a
> particular flavour thereof.
> 
> It just seems to me we're going to end up with a gazillion flags,
> because DMA_ATTR_SYNC_ON_WRITE isn't quite the same semantic as what
> Power can do. So we'll have to add
> DMA_ATTR_SYNC_ON_WRITE_WRT_OTHER_SYNC_ON_WRITE_MAPPINGS_ONLY and so
> on. Or, we end with subtle driver bugs because the semantics aren't
> clear across platforms.

I agree. Just have to sort those issues out as people come up with them.

> I guess I think the attributes should either be truly arch-specific,
> ie. DMA_ATTR_IA64_SYNC_ON_WRITE. Or we need to come up with some well
> defined, and architecture neutral semantics for what the flags mean.

The latter. I have no intention of adding arch specific flags that
only mean something on one arch. The intent here is to enable
correct functionality on specific arches without impacting
performance on arches that don't need those "attributes".

hth,
grant

> 
> cheers

  reply	other threads:[~2008-02-29 18:25 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-28  3:24 akepner
2008-02-29  2:45 ` Michael Ellerman
2008-02-29 18:25   ` Grant Grundler [this message]
2008-02-29 18:37     ` James Bottomley
2008-03-01  2:56       ` Benjamin Herrenschmidt
2008-03-01  3:11         ` Jesse Barnes
2008-03-01  7:18         ` Grant Grundler
2008-03-05 18:13       ` akepner
2008-03-05 19:02         ` Jesse Barnes
2008-03-06  6:01         ` Michael Ellerman
2008-03-12  1:19           ` akepner
2008-03-14  4:13             ` Grant Grundler
2008-03-14  4:30             ` Michael Ellerman
2008-03-14  5:21               ` Grant Grundler
2008-03-14 16:40                 ` Jesse Barnes
2008-03-18  1:08                 ` Michael Ellerman
2008-03-20  0:32               ` akepner
2008-02-29 21:23   ` akepner

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=20080229182504.GA18102@colo.lackof.org \
    --to=grundler@parisc-linux.org \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=akepner@sgi.com \
    --cc=benh@kernel.crashing.org \
    --cc=davem@davemloft.net \
    --cc=jbarnes@virtuousgeek.org \
    --cc=jes@sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael@ellerman.id.au \
    --cc=randy.dunlap@oracle.com \
    --cc=rdreier@cisco.com \
    --cc=tony.luck@intel.com \
    --subject='Re: [PATCH 1/3 v3] dma: document dma_{un}map_{single|sg}_attrs() interface' \
    /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

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).