Re: [PATCH] staging: ion: remove from the tree

[John Stultz]

On Fri, Aug 28, 2020 at 1:05 AM Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, Aug 27, 2020 at 11:54:12AM -0700, John Stultz wrote:
> > On Thu, Aug 27, 2020 at 10:17 AM Greg Kroah-Hartman
> > <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > > On Thu, Aug 27, 2020 at 10:31:41PM +0530, Amit Pundir wrote:
> > > > I don't know what is the right thing to do here. I just want to
> > > > highlight that AOSP's audio (codec2) HAL depends on the ION system
> > > > heap and it will break AOSP for people who boot mainline on their
> > > > devices, even for just testing purpose like we do in Linaro. Right now
> > > > we need only 1 (Android specific out-of-tree) patch to boot AOSP with
> > > > mainline and Sumit is already trying to upstream that vma naming
> > > > patch. Removal of in-kernel ION, will just add more to that delta.
> > >
> > > As AOSP will continue to rely on ION after December of this year, all
> > > you are doing is postponing the inevitable a few more months.
> > >
> > > Push back on the Android team to fix up the code to not use ION, they
> > > know this needs to happen.
> >
> > The point though, is your main premise that no one is using this isn't true.
>
> They are using the version of ion in the Android kernel tree, yes, as it
> has new features that many people are relying on.
>
> The version that is currently in the kernel tree is crippled, and maybe
> works for some use cases, but not the majority, right?

So my understanding is the Android Common Kernel tree version was
mostly reworked to allow heaps as modules, and allowed heaps to have
their own exporter logic (not unlike how dmabuf heaps do it). The main
allocation interface is maybe slightly tweaked for out-of-tree vendor
heaps, but doesn't affect the in-staging heaps. There's also a few
optimizations we have skipped taking upstream. So yes, there are
differences, but I don't feel your characterization is quite accurate.


> > I'm actively working with Hridya and folks on the codec2 HAL side to
> > transition this on the userland side:
> >   https://android-review.googlesource.com/c/platform/frameworks/av/+/1368918/3
> >
> > I'd like AOSP to not use ION after September (though being external I
> > can't promise anything), much less continuing after December.
>
> The android team has said they will be dropping ION use for the "next"
> Android release, which is sometime next year from what I recall.
> December is probably not going to happen :)

AOSP is what the next Android release forks off of, so it needs to be
fixed first.

> > I want this migration to happen as much as anyone.  But I'd prefer to
> > keep ION in staging until after the LTS is announced. Having both
> > around helps development for the transition, which helps us have a
> > reliable solution, which helps vendors to migrate and be able to do
> > comparative performance testing.
>
> I don't understand what having this in the "next" kernel helps us with
> here.  And I would really really prefer to NOT have an outdated version
> of this code in a kernel tree that I am going to have to support for the
> next X number of years, when no one is using that version of the driver.
>
> What is this LTS fixation to keep this code around for?  Who does it
> help?

Vendors usually target LTS releases for their hardware bringups.
Having a LTS release with both ION and DMA BUF Heaps helps them
validate their old ION solution as performant, and then migrate to DMA
BUF Heaps and be able to do performance comparisons holding all other
things equal.

> > I do appreciate that keeping it isn't free, but I also don't feel the
> > chaos-monkey approach here is really motivational in the way you
> > intend.
>
> I don't see it helping anyone to leave this around, except to cause
> merge issues for me, and development issues for other developers.
>
> Anyone who really wants this code, can easily revert the deletion and
> move on and grab the AOSP copy of the code.  That's what they did when
> we deleted other Android features that are still relied on.
>
> Given that the "isn't free" is causing _me_ real pain, and not the
> actual users of this code, I am leaning toward wanting to move that
> pain/cost to those users, for obvious reasons.

Sure. Again, I do understand the desire to remove it, and it's your
right to do so. Keeping the code for an extra year in LTS (over 5.4)
is a cost, so I understand if you drop it. But I'll ask that you make
that judgement clear as the main motivator/rationale of the commit
message, rather than flippantly pretending it's not being used, and
that everyone agrees it has no usefulness to keep around (especially
after we've had this conversation a few times already this year).

thanks
-john