Re: [PATCH v3 2/2] staging: ion: create one device entry per heap

[Laura Abbott]

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