From: "Andrew F. Davis" <afd@ti.com>
To: Laura Abbott <labbott@redhat.com>, Brian Starkey <Brian.Starkey@arm.com>
Cc: "Sumit Semwal" <sumit.semwal@linaro.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Arve Hjønnevåg" <arve@android.com>,
"devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>, nd <nd@arm.com>
Subject: Re: [PATCH 11/14] staging: android: ion: Allow heap name to be null
Date: Mon, 21 Jan 2019 08:30:37 -0600 [thread overview]
Message-ID: <5c8fecd6-e8d5-127f-8271-8f91d8bfeb49@ti.com> (raw)
In-Reply-To: <042dd3b1-9a41-2a76-270d-1c992d782c40@redhat.com>
On 1/18/19 1:53 PM, Laura Abbott wrote:
> On 1/16/19 9:12 AM, Andrew F. Davis wrote:
>> On 1/16/19 9:28 AM, Brian Starkey wrote:
>>> Hi Andrew,
>>>
>>> On Fri, Jan 11, 2019 at 12:05:20PM -0600, Andrew F. Davis wrote:
>>>> The heap name can be used for debugging but otherwise does not seem
>>>> to be required and no other part of the code will fail if left NULL
>>>> except here. We can make it required and check for it at some point,
>>>> for now lets just prevent this from causing a NULL pointer exception.
>>>
>>> I'm not so keen on this one. In the "new" API with heap querying, the
>>> name string is the only way to identify the heap. I think Laura
>>> mentioned at XDC2017 that it was expected that userspace should use
>>> the strings to find the heap they want.
>>>
>>
>> Right now the names are only for debug. I accidentally left the name
>> null once and got a kernel crash. This is the only spot where it is
>> needed so I fixed it up. The other option is to make the name mandatory
>> and properly error out, I don't want to do that right now until the
>> below discussion is had to see if names really do matter or not.
>>
>
> Yes, the heap names are part of the query API and are the expected
> way to identify individual heaps for the API at the moment so having
> a null heap name is incorrect. The heap name seemed like the best way
> to identify heaps to userspace but if you have an alternative proposal
> I'd be interested.
>
Not sure I have a better proposal right now, I'll re-work this patch to
force heap names to be populated before ion_device_add_heap() instead.
(do you think that function name is now is a misnomer? how do you feel
about renaming that to just ion_add_heap()?)
Andrew
> Thanks,
> Laura
>
>>
>
>
next prev parent reply other threads:[~2019-01-21 14:30 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-11 18:05 [PATCH 00/14] Misc ION cleanups and adding unmapped heap Andrew F. Davis
2019-01-11 18:05 ` [PATCH 01/14] staging: android: ion: Add proper header information Andrew F. Davis
2019-01-11 18:05 ` [PATCH 02/14] staging: android: ion: Remove empty ion_ioctl_dir() function Andrew F. Davis
2019-01-11 18:05 ` [PATCH 03/14] staging: android: ion: Merge ion-ioctl.c into ion.c Andrew F. Davis
2019-01-11 18:05 ` [PATCH 04/14] staging: android: ion: Remove leftover comment Andrew F. Davis
2019-01-11 18:05 ` [PATCH 05/14] staging: android: ion: Remove struct ion_platform_heap Andrew F. Davis
2019-01-11 18:05 ` [PATCH 06/14] staging: android: ion: Fixup some white-space issues Andrew F. Davis
2019-01-11 18:05 ` [PATCH 07/14] staging: android: ion: Sync comment docs with struct ion_buffer Andrew F. Davis
2019-01-11 18:05 ` [PATCH 08/14] staging: android: ion: Remove base from ion_carveout_heap Andrew F. Davis
2019-01-11 18:05 ` [PATCH 09/14] staging: android: ion: Remove base from ion_chunk_heap Andrew F. Davis
2019-01-11 18:05 ` [PATCH 10/14] staging: android: ion: Remove unused headers Andrew F. Davis
2019-01-11 18:05 ` [PATCH 11/14] staging: android: ion: Allow heap name to be null Andrew F. Davis
2019-01-16 15:28 ` Brian Starkey
2019-01-16 17:12 ` Andrew F. Davis
2019-01-18 19:53 ` Laura Abbott
2019-01-21 14:30 ` Andrew F. Davis [this message]
2019-01-11 18:05 ` [PATCH 12/14] staging: android: ion: Declare helpers for carveout and chunk heaps Andrew F. Davis
2019-01-18 9:59 ` Greg Kroah-Hartman
2019-01-18 16:09 ` Andrew F. Davis
2019-01-18 19:54 ` Laura Abbott
2019-01-11 18:05 ` [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap Andrew F. Davis
2019-01-14 17:13 ` Liam Mark
2019-01-15 15:44 ` Andrew F. Davis
2019-01-15 17:45 ` Liam Mark
2019-01-15 18:38 ` Andrew F. Davis
2019-01-15 18:40 ` Andrew F. Davis
2019-01-16 15:19 ` Brian Starkey
2019-01-16 17:05 ` Andrew F. Davis
2019-01-16 22:54 ` Liam Mark
2019-01-17 16:25 ` Andrew F. Davis
2019-01-18 1:11 ` Liam Mark
2019-01-18 17:16 ` Andrew F. Davis
2019-01-21 11:22 ` Brian Starkey
2019-01-21 21:21 ` Andrew F. Davis
2019-01-22 17:33 ` Sumit Semwal
2019-01-23 16:51 ` Andrew F. Davis
2019-01-23 17:11 ` Brian Starkey
2019-01-24 16:04 ` Andrew F. Davis
2019-01-24 16:44 ` Brian Starkey
2019-02-19 21:37 ` Laura Abbott
[not found] ` <CAO_48GHZPkE8Or_dB6CVMi5Jv5eufE_Z36MT7ztJwTqTzkTpKA@mail.gmail.com>
2019-01-23 17:09 ` Andrew F. Davis
2019-01-22 22:56 ` Liam Mark
2019-01-21 20:11 ` Liam Mark
2019-01-15 19:05 ` Laura Abbott
2019-01-16 16:17 ` Andrew F. Davis
2019-01-16 22:48 ` Liam Mark
2019-01-17 16:13 ` Andrew F. Davis
2019-01-18 1:04 ` Liam Mark
2019-01-18 16:50 ` Andrew F. Davis
2019-01-18 21:43 ` Liam Mark
2019-01-21 15:15 ` Andrew F. Davis
2019-01-21 20:04 ` Liam Mark
2019-01-18 20:31 ` Laura Abbott
2019-01-18 20:43 ` Andrew F. Davis
2019-01-18 20:46 ` Laura Abbott
2019-01-19 10:11 ` Christoph Hellwig
2019-01-17 9:02 ` Christoph Hellwig
2019-01-11 18:05 ` [PATCH 14/14] staging: android: ion: Add UNMAPPED heap type and helper Andrew F. Davis
2019-01-15 2:32 ` Laura Abbott
2019-01-15 15:58 ` Andrew F. Davis
2019-01-15 18:43 ` Laura Abbott
2019-01-15 19:11 ` Laura Abbott
2019-01-16 16:18 ` Andrew F. Davis
2019-01-15 2:39 ` [PATCH 00/14] Misc ION cleanups and adding unmapped heap Laura Abbott
2019-01-15 17:47 ` Andrew F. Davis
2019-01-15 18:58 ` Laura Abbott
2019-01-16 16:05 ` Andrew F. Davis
2019-01-18 20:19 ` Laura Abbott
2019-01-21 14:58 ` Andrew F. Davis
2019-01-18 9:56 ` Greg Kroah-Hartman
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=5c8fecd6-e8d5-127f-8271-8f91d8bfeb49@ti.com \
--to=afd@ti.com \
--cc=Brian.Starkey@arm.com \
--cc=arve@android.com \
--cc=devel@driverdev.osuosl.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=gregkh@linuxfoundation.org \
--cc=labbott@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=nd@arm.com \
--cc=sumit.semwal@linaro.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).