Subject: Re: [PATCH v3 2/2] staging: ion: create one device entry per heap
To: Greg KH <gregkh@linuxfoundation.org>,
        Benjamin Gaignard <benjamin.gaignard@linaro.org>,
        sumit.semwal@linaro.org, arve@android.com, riandrews@android.com,
        broonie@kernel.org, dan.carpenter@oracle.com,
        devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org
References: <1505897105-23721-1-git-send-email-benjamin.gaignard@linaro.org>
 <1505897105-23721-3-git-send-email-benjamin.gaignard@linaro.org>
 <3dbd167f-c381-8451-5861-3788634885b7@redhat.com>
 <20170926050914.pdc64yafdhfg634a@phenom.ffwll.local>
 <20170926065627.GD6250@kroah.com>
From: Laura Abbott <labbott@redhat.com>
Message-ID: <14f79571-73c5-22c3-2871-43b744bac0d5@redhat.com>
Date: Tue, 26 Sep 2017 10:52:49 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.0
MIME-Version: 1.0
In-Reply-To: <20170926065627.GD6250@kroah.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2352
Lines: 55

On 09/25/2017 11:56 PM, Greg KH wrote:
> On Tue, Sep 26, 2017 at 07:09:14AM +0200, Daniel Vetter wrote:
>> On Mon, Sep 25, 2017 at 11:26:27AM -0700, Laura Abbott wrote:
>>> On 09/20/2017 01:45 AM, Benjamin Gaignard wrote:
>>>> Instead a getting one common device "/dev/ion" for
>>>> all the heaps this patch allow to create one device
>>>> entry ("/dev/ionX") per heap.
>>>> Getting an entry per heap could allow to set security rules
>>>> per heap and global ones for all heaps.
>>>>
>>>> Allocation requests will be only allowed if the mask_id
>>>> match with device minor.
>>>> Query request could be done on any of the devices.
>>>> Deivce node major will also change and that may impact init scripts.
>>>>
>>>
>>> We should start Cc linux-api for future changes since we're going
>>> to have more than just /dev/ion.
>>>
>>> Thinking about this some more, I think we need to allow backwards
>>> compatibility. It's just not feasible to keep throwing workarounds
>>> into userspace if we can avoid it. I'd propose keeping the old /dev/ion
>>> misc interface available under a Kconfig and then creating the new
>>> split heaps in parallel.
>>>
>>> On a somewhat related note, there was some interest in possibly
>>> having a sysfs interface for Ion long term. I don't think this
>>> needs to happen right now but I'd like whatever we do to not
>>> make adding that harder.
>>
>> Not sure sysfs is a good idea for allocating buffers. The backlight
>> interface is in sysfs, which defacto makes it a root-only interface.
>> Distros really hate that, because it makes unpriviledged X/wayland really
>> hard to pull of. Passing a device file otoh from logind to the compositor
>> is dead simple (and how X et al get at e.g. the drm/input devices
>> already).
> 
> sysfs is not a good idea for allocating buffers.  We already have some
> out-of-tree drm drivers doing horrid things in sysfs in ways that
> totally abuse that api, and vendors have to do crazy things with selinux
> rules to try to lock it down because of that.  A device node is fine, we
> are used to that for graphics stuff :)
> 
> thanks,
> 
> greg k-h
> 

I wasn't thinking of sysfs for allocation, this was for exposure of
other Ion information that might make more sense than debugfs. Like
I said, this was mostly forward thinking to make sure we aren't
stuck later.

Thanks,
Laura