linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eli Billauer <eli.billauer@gmail.com>
To: Joerg Roedel <joro@8bytes.org>
Cc: Tejun Heo <tj@kernel.org>, Shuah Khan <shuah.kh@samsung.com>,
	devel@driverdev.osuosl.org, gregkh@linuxfoundation.org,
	bhelgaas@google.com, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org, iommu@lists.linux-foundation.org,
	discuss@x86-64.org
Subject: Re: [PATCH v2 1/4] dma-mapping: Add devm_ interface for dma_map_single()
Date: Fri, 06 Jun 2014 14:45:06 +0300	[thread overview]
Message-ID: <5391A9C2.3040602@gmail.com> (raw)
In-Reply-To: <20140604212525.GE23880@8bytes.org>

Hello Joerg.


On 05/06/14 00:25, Joerg Roedel wrote:
>
> What you are trying to do should work with dma_alloc_noncoherent(). The
> API allows partial syncs on this memory, so you should be fine.
>    
Please try to put yourself in my position: I have a driver that I care 
about, which works fine for a few years. It's based upon 
dma_map_single(), which seems to be the common way to get non-coherent 
memory, even for the driver's entire lifespan. I realize that 
dma_alloc_* was the intended way to do it, but fact is that dma_map_* 
has become the common choice.

Now I need to switch to dma_alloc_noncoherent(), which isn't used by 
many drivers, it seems. It should work the same, but there's always the 
worry if I'll cover all the corners. So will I take this risk of a nasty 
DMA bug on some esoteric platform, just to cut some lines in the code?

And if I choose to keep the unmanaged dma_map_single(), maybe I'll mess 
up if I convert other allocations to the managed API? Hmmm, maybe it's 
best to forget all about it.
> The problem with a devm variant of dma_map_* is that it is too easy to
> misuse or to use it wrong so that a driver could eat up all available
> DMA handles on some platforms.
>    
I suppose you mean that those who use dma_map_* for a one-off DMA 
session will drop the unmapping, thinking that it's "managed anyhow"...? 
Well, you can say that about any of the managed functions. For example, 
when devm_kzalloc() was introduced, someone maybe argued that people 
would drop kfree()s where they shouldn't, causing memory leaks.

So I think it boils down to whether devres is a good idea or not. 
Someone who thinks it's bad, will reject any new API, referring to 
memory efficiency, additional causes of failure and the risk of 
misleading the herd.

But if devres is to become commonly used in the future, I think drop-in 
replacements are necessary. In my opinion, telling people to adopt 
another methodology (e.g. dma_alloc_noncoherent vs. mapping), even if 
functionally equivalent, is a good way to make sure devres is never adopted.

Regards,
    Eli
> 	Joerg
>
>
>    



  reply	other threads:[~2014-06-06 11:46 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-01  7:01 [PATCH v2 0/4] devres: dma-mapping: Introducing new functions Eli Billauer
2014-06-01  7:01 ` [PATCH v2 1/4] dma-mapping: Add devm_ interface for dma_map_single() Eli Billauer
2014-06-03 21:24   ` Shuah Khan
2014-06-03 23:39     ` Joerg Roedel
2014-06-04 14:04       ` Tejun Heo
2014-06-04 14:12         ` Joerg Roedel
2014-06-04 14:14           ` Tejun Heo
2014-06-04 15:03             ` Eli Billauer
2014-06-04 21:25               ` Joerg Roedel
2014-06-06 11:45                 ` Eli Billauer [this message]
2014-06-06 16:01                   ` Greg KH
2014-06-06 16:21                     ` Eli Billauer
2014-06-06 17:02                       ` Shuah Khan
2014-06-07 11:23                         ` Eli Billauer
2014-06-01  7:01 ` [PATCH v2 2/4] dma-mapping: Add devm_ interface for dma_map_single_attrs() Eli Billauer
2014-06-01  7:01 ` [PATCH v2 3/4] dma-mapping: pci: Add devm_ interface for pci_map_single Eli Billauer
2014-06-01  7:01 ` [PATCH v2 4/4] staging: xillybus: Use devm_ API for memory allocation and DMA mapping Eli Billauer

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=5391A9C2.3040602@gmail.com \
    --to=eli.billauer@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=discuss@x86-64.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=joro@8bytes.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=shuah.kh@samsung.com \
    --cc=tj@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).