Re: [PATCH v2 0/2] iommu: Allow passing custom allocators to pgtable drivers

[Boris Brezillon <boris.brezillon@collabora.com>]

On Mon, 20 Nov 2023 10:46:04 -0400
Jason Gunthorpe <jgg@ziepe.ca> wrote:

> On Mon, Nov 20, 2023 at 03:38:38PM +0100, Boris Brezillon wrote:
> 
> > > At LPC there was quite a lot if interest in improving the io page
> > > table stuff to work better. Based on that I'm even more against adding
> > > an external allocator at this time. :\  
> > 
> > I'm sure this has been discussed with the IOMMU maintainers, but I'd
> > really like to hear it from them if you don't mind. Especially since,
> > as I mentioned, the custom allocator idea comes from Robin, not me...  
> 
> It is a community effort - this is my opinion. We now understand this
> io page table area needs a lot of work to meet all the new needs. I'm
> against hacking it up like this and making that work harder.

Consider it a hack if you like, but the thing is pretty self-contained,
and I doubt it will get to a point where things become unmaintainable
before you get the generic caching system working. Not to mention
Gaurav's use case, which can't really be solved with your caching
solution.

> 
> > v1, was a sign this was good to go. If the final call is to reject the
> > custom allocator approach, and there's no updates on your generic
> > solution in the next couple weeks, I'll probably copy the pgtable code
> > in panthor so I can implement my own stuff, and later switch back to
> > the generic io-pgtable implementation when things have settled down.  
> 
> That's horrible, DRM should refuse that too,

Most GPU drivers have their own MMU/pgtable logic, partly because they
are the only users, and also because most of them delegate the page
table updates to the GPU (page tables leave in VRAM). I've actually
been recommended to go for this approach when I submitted the first
version of the driver, but after further discussions with Robin and
Rob we agreed this was probably not a good idea (because the code
exists already, and having a second implementation is never a good
thing, and also because what's needed for discrete GPUs with VRAM
doesn't necessarily apply to GPUs that share the memory with the rest
of the system).

> yet I expect you will
> probably end up merging like that.
> 
> I would not expect a resolution in a few weeks.

OOC, what's the time frame?

> 
> > Nothing against you or your solution (I actually commit to move to the
> > new approach when it's ready), but we have different priorities, and I
> > can't really delay the driver merging by several months unless there's
> > a very good reason to do it.  
> 
> Welcome to Linux, nothing runs fast :(

I've maintained a subsystem for a few years, so I know what it means
to contribute upstream and ask or being asked to extend the core to do
something the right way. I'm generally patient and try to follow
maintainers' recommendations, but there's some point where waiting is
no more an option, and you have to be more pragmatic, because there's
always a better way of doing things, and perfection is often the enemy
of good...