[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5c8fecd6-e8d5-127f-8271-8f91d8bfeb49@ti.com>
Date: Mon, 21 Jan 2019 08:30:37 -0600
From: "Andrew F. Davis" <afd@...com>
To: Laura Abbott <labbott@...hat.com>,
Brian Starkey <Brian.Starkey@....com>
CC: Sumit Semwal <sumit.semwal@...aro.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Arve Hjønnevåg <arve@...roid.com>,
"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
nd <nd@....com>
Subject: Re: [PATCH 11/14] staging: android: ion: Allow heap name to be null
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
>
>>
>
>
Powered by blists - more mailing lists