linux-kernel - Re: [PATCH v2] staging: ion: Add a default struct device for cma heap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 7 Aug 2015 16:09:12 -0700
From:	Laura Abbott <labbott@...hat.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Feng Tang <feng.tang@...el.com>
Cc:	Michal Nazarewicz <mina86@...a86.com>,
	John Stultz <john.stultz@...aro.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	LKML <linux-kernel@...r.kernel.org>,
	Sumit Semwal <sumit.semwal@...aro.org>
Subject: Re: [PATCH v2] staging: ion: Add a default struct device for cma heap

On 08/07/2015 11:05 AM, Greg Kroah-Hartman wrote:
> On Fri, Aug 07, 2015 at 11:50:04PM +0800, Feng Tang wrote:
>> On Fri, Aug 07, 2015 at 04:48:28PM +0200, Michal Nazarewicz wrote:
>>> On Fri, Aug 07 2015, Feng Tang wrote:
>>>> As I described above, the dummy struct device is only needed for
>>>> dma request, its lifetime is align with the cma_heap itself.
>>>
>>> Again, this is from perspective of someone who is unfamiliar with ION,
>>> but perhaps a viable solution is to bypass DMA API and just call
>>> cma_alloc directly?
>>
>> For ion cma heap, the buffer allocation func ion_cma_allocate() will
>> call dma_alloc_coherent(dev, ...). And dma_alloc_coherent() is
>> implemented by each architeture(arm/m68k/x86 etc), and many Arch's
>> implementation doesn't use cma, but use alloc_pages() like APIs.
>> So I'm afraid we can't direcly call cma_alloc directly here.
>
> Ick.  But using a "fake" struct device here, for no real reason,
> makes me very nervous that you are going to hit a codepath somewhere
> that assumes this is a "real" struct device and tries to do something
> with it (dev_printk(), look up what bus it is on, change the name of it,
> etc.)  Trying to fake out the subsystem in this manner is a sign that
> something is really wrong here.
>
> Please either make this a real device, or fix up the api to not need
> this type of thing.
>

I think this issue represents one of the many current issues with Ion.
When the void * == struct dev was added, everything was working off of
board files. We now have devicetree which makes the device association
even more awkward to pull off. Every vendor out there is doing something
different right now so the assertion in the commit text about 'normal'
is not true; existing code has managed to work with the (not super great)
API.

There is going to be an Ion session at Plumbers in a few weeks. I'd like
to propose holding off on merging anything until after plumbers when
there can be some more discussion about what would be a reasonable API,
taking into consideration the points brought up in this patch series.

Thanks,
Laura
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/