Re: [PATCH 00/10] HMM updates for 5.1

[Dan Williams]

On Mon, Mar 18, 2019 at 11:55 AM Jerome Glisse <jglisse@xxxxxxxxxx> wrote:
>
> On Mon, Mar 18, 2019 at 11:30:15AM -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 10:04 AM Jerome Glisse <jglisse@xxxxxxxxxx> wrote:
> > >
> > > On Wed, Mar 13, 2019 at 09:10:04AM -0700, Andrew Morton wrote:
> > > > On Tue, 12 Mar 2019 21:27:06 -0400 Jerome Glisse <jglisse@xxxxxxxxxx> wrote:
> > > >
> > > > > Andrew you will not be pushing this patchset in 5.1 ?
> > > >
> > > > I'd like to.  It sounds like we're converging on a plan.
> > > >
> > > > It would be good to hear more from the driver developers who will be
> > > > consuming these new features - links to patchsets, review feedback,
> > > > etc.  Which individuals should we be asking?  Felix, Christian and
> > > > Jason, perhaps?
> > > >
> > >
> > > So i am guessing you will not send this to Linus ? Should i repost ?
> > > This patchset has 2 sides, first side is just reworking the HMM API
> > > to make something better in respect to process lifetime. AMD folks
> > > did find that helpful [1]. This rework is also necessary to ease up
> > > the convertion of ODP to HMM [2] and Jason already said that he is
> > > interested in seing that happening [3]. By missing 5.1 it means now
> > > that i can not push ODP to HMM in 5.2 and it will be postpone to 5.3
> > > which is also postoning other work ...
> > >
> > > The second side is it adds 2 new helper dma map and dma unmap both
> > > are gonna be use by ODP and latter by nouveau (after some other
> > > nouveau changes are done). This new functions just do dma_map ie:
> > >     hmm_dma_map() {
> > >         existing_hmm_api()
> > >         for_each_page() {
> > >             dma_map_page()
> > >         }
> > >     }
> > >
> > > Do you want to see anymore justification than that ?
> >
> > Yes, why does hmm needs its own dma mapping apis? It seems to
> > perpetuate the perception that hmm is something bolted onto the side
> > of the core-mm rather than a native capability.
>
> Seriously ?

Yes.

> Kernel is fill with example where common code pattern that are not
> device specific are turn into helpers and here this is exactly what
> it is. A common pattern that all device driver will do which is turn
> into a common helper.

Yes, but we also try not to introduce thin wrappers around existing
apis. If the current dma api does not understand some hmm constraint
I'm questioning why not teach the dma api that constraint and make it
a native capability rather than asking the driver developer to
understand the rules about when to use dma_map_page() vs
hmm_dma_map().

For example I don't think we want to end up with more headers like
include/linux/pci-dma-compat.h.

> Moreover this allow to share the same error code handling accross
> driver when mapping one page fails. So this avoid the needs to
> duplicate same boiler plate code accross different drivers.
>
> Is code factorization not a good thing ? Should i duplicate every-
> thing in every single driver ?

I did not ask for duplication, I asked why is it not more deeply integrated.

> If that's not enough, this will also allow to handle peer to peer
> and i posted patches for that [1] and again this is to avoid
> duplicating common code accross different drivers.

I went looking for the hmm_dma_map() patches on the list but could not
find them, so I was reacting to the "This new functions just do
dma_map", and wondered if that was the full extent of the
justification.

> It does feel that you oppose everything with HMM in its name just
> because you do not like it. It is your prerogative to not like some-
> thing but you should propose something that achieve the same result
> instead of constantly questioning every single comma.

I respect what you're trying to do, if I didn't I wouldn't bother
responding. Please don't put words in my mouth. I think it was
Churchill who said "if two people agree all the time, one of them is
redundant". You're raising questions with HMM that identify real gaps
in Linux memory management relative to new hardware capabilities, I
also think it is reasonable to question how the gaps are filled.