2019-07-17 07:42:07

by Pavel Machek

[permalink] [raw]
Subject: HMM_MIRROR has less than useful help text

Hi!

Commit c0b124054f9e42eb6da545a10fe9122a7d7c3f72 has very nice commit
message, explaining what HMM_MIRROR is and when it is
needed. Unfortunately, it did not make it into Kconfig help:

CONFIG_HMM_MIRROR:

Select HMM_MIRROR if you want to mirror range of the CPU page table of
a
process into a device page table. Here, mirror means "keep
synchronized".
Prerequisites: the device must provide the ability to write-protect
its
page tables (at PAGE_SIZE granularity), and must be able to recover
from
the resulting potential page faults.

Could that be fixed?

This is key information for me:

# This is a heterogeneous memory management (HMM) process address space
# mirroring.
# This is useful for NVidia GPU >= Pascal, Mellanox IB >= mlx5 and more
# hardware in the future.

Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (969.00 B)
signature.asc (188.00 B)
Digital signature
Download all attachments

2019-07-18 11:05:18

by Balbir Singh

[permalink] [raw]
Subject: Re: HMM_MIRROR has less than useful help text

On Wed, Jul 17, 2019 at 5:41 PM Pavel Machek <[email protected]> wrote:
>
> Hi!
>
> Commit c0b124054f9e42eb6da545a10fe9122a7d7c3f72 has very nice commit
> message, explaining what HMM_MIRROR is and when it is
> needed. Unfortunately, it did not make it into Kconfig help:
>
> CONFIG_HMM_MIRROR:
>
> Select HMM_MIRROR if you want to mirror range of the CPU page table of
> a
> process into a device page table. Here, mirror means "keep
> synchronized".
> Prerequisites: the device must provide the ability to write-protect
> its
> page tables (at PAGE_SIZE granularity), and must be able to recover
> from
> the resulting potential page faults.
>
> Could that be fixed?
>
> This is key information for me:
>
> # This is a heterogeneous memory management (HMM) process address space
> # mirroring.
> # This is useful for NVidia GPU >= Pascal, Mellanox IB >= mlx5 and more
> # hardware in the future.
>

That seems like a reasonable request

Balbir

> Thanks,
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2019-07-18 14:27:45

by Dan Williams

[permalink] [raw]
Subject: Re: HMM_MIRROR has less than useful help text

On Thu, Jul 18, 2019 at 4:04 AM Balbir Singh <[email protected]> wrote:
>
> On Wed, Jul 17, 2019 at 5:41 PM Pavel Machek <[email protected]> wrote:
> >
> > Hi!
> >
> > Commit c0b124054f9e42eb6da545a10fe9122a7d7c3f72 has very nice commit
> > message, explaining what HMM_MIRROR is and when it is
> > needed. Unfortunately, it did not make it into Kconfig help:
> >
> > CONFIG_HMM_MIRROR:
> >
> > Select HMM_MIRROR if you want to mirror range of the CPU page table of
> > a
> > process into a device page table. Here, mirror means "keep
> > synchronized".
> > Prerequisites: the device must provide the ability to write-protect
> > its
> > page tables (at PAGE_SIZE granularity), and must be able to recover
> > from
> > the resulting potential page faults.
> >
> > Could that be fixed?
> >
> > This is key information for me:
> >
> > # This is a heterogeneous memory management (HMM) process address space
> > # mirroring.
> > # This is useful for NVidia GPU >= Pascal, Mellanox IB >= mlx5 and more
> > # hardware in the future.
> >
>
> That seems like a reasonable request

Hi Pavel, care to send a patch?

2019-07-18 16:37:05

by Pavel Machek

[permalink] [raw]
Subject: Re: HMM_MIRROR has less than useful help text

On Thu 2019-07-18 07:25:42, Dan Williams wrote:
> On Thu, Jul 18, 2019 at 4:04 AM Balbir Singh <[email protected]> wrote:
> >
> > On Wed, Jul 17, 2019 at 5:41 PM Pavel Machek <[email protected]> wrote:
> > >
> > > Hi!
> > >
> > > Commit c0b124054f9e42eb6da545a10fe9122a7d7c3f72 has very nice commit
> > > message, explaining what HMM_MIRROR is and when it is
> > > needed. Unfortunately, it did not make it into Kconfig help:
> > >
> > > CONFIG_HMM_MIRROR:
> > >
> > > Select HMM_MIRROR if you want to mirror range of the CPU page table of
> > > a
> > > process into a device page table. Here, mirror means "keep
> > > synchronized".
> > > Prerequisites: the device must provide the ability to write-protect
> > > its
> > > page tables (at PAGE_SIZE granularity), and must be able to recover
> > > from
> > > the resulting potential page faults.
> > >
> > > Could that be fixed?
> > >
> > > This is key information for me:
> > >
> > > # This is a heterogeneous memory management (HMM) process address space
> > > # mirroring.
> > > # This is useful for NVidia GPU >= Pascal, Mellanox IB >= mlx5 and more
> > > # hardware in the future.
> > >
> >
> > That seems like a reasonable request
>
> Hi Pavel, care to send a patch?

I hoped patch author would fix up their code. I'm not HMM expert, he
should be...

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (1.47 kB)
signature.asc (188.00 B)
Digital signature
Download all attachments

2019-07-19 01:33:35

by John Hubbard

[permalink] [raw]
Subject: [PATCH] mm/Kconfig: additional help text for HMM_MIRROR option

From: John Hubbard <[email protected]>

The HMM_MIRROR option in Kconfig is a little underdocumented and
mysterious, and leaves people wondering whether to enable it.

Add text explaining just a little bit more about HMM, and also
mention which hardware would benefit from having HMM_MIRROR
enabled.

Suggested-by: Pavel Machek <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: Jerome Glisse <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Signed-off-by: John Hubbard <[email protected]>
---

Hi Pavel and all, does this help? I've tried to capture the key missing bits
of documentation, but still keep it small, for Kconfig.

thanks,
John Hubbard
NVIDIA

mm/Kconfig | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/mm/Kconfig b/mm/Kconfig
index 56cec636a1fc..2fcb92e7f696 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -681,11 +681,18 @@ config HMM_MIRROR
depends on MMU && 64BIT
select MMU_NOTIFIER
help
- Select HMM_MIRROR if you want to mirror range of the CPU page table of a
- process into a device page table. Here, mirror means "keep synchronized".
- Prerequisites: the device must provide the ability to write-protect its
- page tables (at PAGE_SIZE granularity), and must be able to recover from
- the resulting potential page faults.
+ This is Heterogeneous Memory Management (HMM) process address space
+ mirroring.
+
+ HMM_MIRROR provides a way to mirror ranges of the CPU page tables
+ of a process into a device page table. Here, mirror means "keep
+ synchronized". Prerequisites: the device must provide the ability
+ to write-protect its page tables (at PAGE_SIZE granularity), and
+ must be able to recover from the resulting potential page faults.
+
+ Select HMM_MIRROR if you have hardware that meets the above
+ description. An early, partial list of such hardware is:
+ an NVIDIA GPU >= Pascal, Mellanox IB >= mlx5, or an AMD GPU.

config DEVICE_PRIVATE
bool "Unaddressable device memory (GPU memory, ...)"
--
2.22.0

2019-07-19 04:36:03

by Ira Weiny

[permalink] [raw]
Subject: Re: [PATCH] mm/Kconfig: additional help text for HMM_MIRROR option

On Thu, Jul 18, 2019 at 06:32:53PM -0700, [email protected] wrote:
> From: John Hubbard <[email protected]>
>
> The HMM_MIRROR option in Kconfig is a little underdocumented and
> mysterious, and leaves people wondering whether to enable it.
>
> Add text explaining just a little bit more about HMM, and also
> mention which hardware would benefit from having HMM_MIRROR
> enabled.
>
> Suggested-by: Pavel Machek <[email protected]>
> Cc: Balbir Singh <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: Jerome Glisse <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Signed-off-by: John Hubbard <[email protected]>
> ---
>
> Hi Pavel and all, does this help? I've tried to capture the key missing bits
> of documentation, but still keep it small, for Kconfig.
>
> thanks,
> John Hubbard
> NVIDIA
>
> mm/Kconfig | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 56cec636a1fc..2fcb92e7f696 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -681,11 +681,18 @@ config HMM_MIRROR
> depends on MMU && 64BIT
> select MMU_NOTIFIER
> help
> - Select HMM_MIRROR if you want to mirror range of the CPU page table of a
> - process into a device page table. Here, mirror means "keep synchronized".
> - Prerequisites: the device must provide the ability to write-protect its
> - page tables (at PAGE_SIZE granularity), and must be able to recover from
> - the resulting potential page faults.
> + This is Heterogeneous Memory Management (HMM) process address space
> + mirroring.
> +
> + HMM_MIRROR provides a way to mirror ranges of the CPU page tables
> + of a process into a device page table. Here, mirror means "keep
> + synchronized". Prerequisites: the device must provide the ability
> + to write-protect its page tables (at PAGE_SIZE granularity), and
> + must be able to recover from the resulting potential page faults.
> +
> + Select HMM_MIRROR if you have hardware that meets the above
> + description. An early, partial list of such hardware is:
> + an NVIDIA GPU >= Pascal, Mellanox IB >= mlx5, or an AMD GPU.

I don't think we want to put device information here. If we want that
information in Kconfig best to put it in the devices themselves. Otherwise
this list will get stale.

Other than that, looks good.

Reviewed-by: Ira Weiny <[email protected]>

Ira

>
> config DEVICE_PRIVATE
> bool "Unaddressable device memory (GPU memory, ...)"
> --
> 2.22.0
>

2019-07-19 05:16:41

by John Hubbard

[permalink] [raw]
Subject: Re: [PATCH] mm/Kconfig: additional help text for HMM_MIRROR option

On 7/18/19 9:34 PM, Ira Weiny wrote:
> On Thu, Jul 18, 2019 at 06:32:53PM -0700, [email protected] wrote:
>> From: John Hubbard <[email protected]>
...
>> + Select HMM_MIRROR if you have hardware that meets the above
>> + description. An early, partial list of such hardware is:
>> + an NVIDIA GPU >= Pascal, Mellanox IB >= mlx5, or an AMD GPU.
>
> I don't think we want to put device information here. If we want that
> information in Kconfig best to put it in the devices themselves. Otherwise
> this list will get stale.
>
> Other than that, looks good.
>
> Reviewed-by: Ira Weiny <[email protected]>
>

Hi Ira, thanks for the review, I'll remove that last sentence. I'll post a v2
with your reviewed by tag, in a new email thread. But first I'll wait to see
if there are other replies.

thanks,
--
John Hubbard
NVIDIA

2019-07-19 05:59:51

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] mm/Kconfig: additional help text for HMM_MIRROR option

On Thu, Jul 18, 2019 at 06:32:53PM -0700, [email protected] wrote:
> + HMM_MIRROR provides a way to mirror ranges of the CPU page tables
> + of a process into a device page table. Here, mirror means "keep
> + synchronized". Prerequisites: the device must provide the ability
> + to write-protect its page tables (at PAGE_SIZE granularity), and
> + must be able to recover from the resulting potential page faults.
> +
> + Select HMM_MIRROR if you have hardware that meets the above
> + description. An early, partial list of such hardware is:
> + an NVIDIA GPU >= Pascal, Mellanox IB >= mlx5, or an AMD GPU.

Nevermind that the Nvidia support is stagaging and looks rather broken,
there is no Mellanox user of this either at this point.

But either way this has no business in a common kconfig help. Just
drop the fine grained details and leave it to the overview.

2019-07-19 10:54:46

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] mm/Kconfig: additional help text for HMM_MIRROR option

On Thu 2019-07-18 22:57:48, Christoph Hellwig wrote:
> On Thu, Jul 18, 2019 at 06:32:53PM -0700, [email protected] wrote:
> > + HMM_MIRROR provides a way to mirror ranges of the CPU page tables
> > + of a process into a device page table. Here, mirror means "keep
> > + synchronized". Prerequisites: the device must provide the ability
> > + to write-protect its page tables (at PAGE_SIZE granularity), and
> > + must be able to recover from the resulting potential page faults.
> > +
> > + Select HMM_MIRROR if you have hardware that meets the above
> > + description. An early, partial list of such hardware is:
> > + an NVIDIA GPU >= Pascal, Mellanox IB >= mlx5, or an AMD GPU.
>
> Nevermind that the Nvidia support is stagaging and looks rather broken,
> there is no Mellanox user of this either at this point.
>
> But either way this has no business in a common kconfig help. Just
> drop the fine grained details and leave it to the overview.

I disagree here. This explains what kind of hardware this is for (very
new). Partial list does not hurt, and I know that I probably don't
need to enable this.

How else am I supposed to know if my computer needs page tables
synchronized?

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (1.36 kB)
signature.asc (188.00 B)
Digital signature
Download all attachments

2019-07-19 11:49:34

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH] mm/Kconfig: additional help text for HMM_MIRROR option

On Fri, Jul 19, 2019 at 12:52:39PM +0200, Pavel Machek wrote:
> On Thu 2019-07-18 22:57:48, Christoph Hellwig wrote:
> > On Thu, Jul 18, 2019 at 06:32:53PM -0700, [email protected] wrote:
> > > + HMM_MIRROR provides a way to mirror ranges of the CPU page tables
> > > + of a process into a device page table. Here, mirror means "keep
> > > + synchronized". Prerequisites: the device must provide the ability
> > > + to write-protect its page tables (at PAGE_SIZE granularity), and
> > > + must be able to recover from the resulting potential page faults.
> > > +
> > > + Select HMM_MIRROR if you have hardware that meets the above
> > > + description. An early, partial list of such hardware is:
> > > + an NVIDIA GPU >= Pascal, Mellanox IB >= mlx5, or an AMD GPU.
> >
> > Nevermind that the Nvidia support is stagaging and looks rather broken,
> > there is no Mellanox user of this either at this point.
> >
> > But either way this has no business in a common kconfig help. Just
> > drop the fine grained details and leave it to the overview.
>
> I disagree here. This explains what kind of hardware this is for (very
> new). Partial list does not hurt, and I know that I probably don't
> need to enable this.
>
> How else am I supposed to know if my computer needs page tables
> synchronized?

It is like MMU_NOTIFIERS, if something needs it, then it will select
it.

Maybe it should just be a hidden kconfig anyhow as there is no reason
to turn it on without also turning on a using driver.

Jason

2019-07-19 12:02:10

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] mm/Kconfig: additional help text for HMM_MIRROR option

On Fri, Jul 19, 2019 at 08:48:53AM -0300, Jason Gunthorpe wrote:
> It is like MMU_NOTIFIERS, if something needs it, then it will select
> it.
>
> Maybe it should just be a hidden kconfig anyhow as there is no reason
> to turn it on without also turning on a using driver.

We can't just select it due to the odd X86_64 || PPC64 dependency.

Which also answers Pavels question: you never really need it, as we
can only use it for optional functionality due to that.

2019-07-19 12:05:57

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] mm/Kconfig: additional help text for HMM_MIRROR option

On Fri 2019-07-19 05:00:43, Christoph Hellwig wrote:
> On Fri, Jul 19, 2019 at 08:48:53AM -0300, Jason Gunthorpe wrote:
> > It is like MMU_NOTIFIERS, if something needs it, then it will select
> > it.
> >
> > Maybe it should just be a hidden kconfig anyhow as there is no reason
> > to turn it on without also turning on a using driver.
>
> We can't just select it due to the odd X86_64 || PPC64 dependency.
>
> Which also answers Pavels question: you never really need it, as we
> can only use it for optional functionality due to that.

Okay, just explain it in the help text :-)..

Alternatively... you can have WANT_HMM_MIRROR option drivers select,
and option HMM_MIRROR which is yes if WANT_HMM_MIRROR && (X86_64 ||
PPC64), no?

Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (915.00 B)
signature.asc (188.00 B)
Digital signature
Download all attachments

2019-07-20 00:34:08

by John Hubbard

[permalink] [raw]
Subject: Re: [PATCH] mm/Kconfig: additional help text for HMM_MIRROR option

On 7/19/19 5:04 AM, Pavel Machek wrote:
> On Fri 2019-07-19 05:00:43, Christoph Hellwig wrote:
>> On Fri, Jul 19, 2019 at 08:48:53AM -0300, Jason Gunthorpe wrote:
>>> It is like MMU_NOTIFIERS, if something needs it, then it will select
>>> it.
>>>
>>> Maybe it should just be a hidden kconfig anyhow as there is no reason
>>> to turn it on without also turning on a using driver.
>>
>> We can't just select it due to the odd X86_64 || PPC64 dependency.
>>
>> Which also answers Pavels question: you never really need it, as we
>> can only use it for optional functionality due to that.
>
> Okay, just explain it in the help text :-)..
>
> Alternatively... you can have WANT_HMM_MIRROR option drivers select,
> and option HMM_MIRROR which is yes if WANT_HMM_MIRROR && (X86_64 ||
> PPC64), no?
>

Yes. This really should be a hidden option that just auto-enables. It's
not ideal to require people to both *find* HMM_MIRROR, *and* figure out
that they need it. (I think it's just this way due to the history of how
HMM got merged--it started off as a kind of experimental sandbox, so
it had it's own config options, to avoid bothering anything else.)

I'll send out a new patch to just auto-select. The WANT_HMM_MIRROR
approach seems accurate, given the (X86_64 || PPC64) complication, probably
after -rc1 is ready (I don't see the ODP code using HMM yet, so that
must not have been merged yet.)

Longer term, I vaguely recall that there is no strong reason preventing
HMM from being made to work on other arches, and am hoping that it was
just done this way to save development time. I don't want to leave it
this way unless there's a good reason to.

thanks,
--
John Hubbard
NVIDIA

2019-07-22 12:50:08

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] mm/Kconfig: additional help text for HMM_MIRROR option

On Mon, Jul 22, 2019 at 08:58:04AM -0300, Jason Gunthorpe wrote:
> No one has given me a satisfactory answer about the restriction
> either.
>
> The only thing this kconfig controls that could possibly be arch
> specific is the page walking code in hmm_range_snapshot and
> related.
>
> Maybe there is/was some arch entanglement there?

The page walking code is supposed to be platform independent.
I did push a branch to the buildbot a few days ago to catch issues,
and the only one found so far is an abuse of pte_index() that can be
trivially fixed. The other thing I noticed is that the use of some
of the p??_none/protnone/present checks seems inconsistent, but I did
not have time to audit that yet.

2019-07-22 15:05:17

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH] mm/Kconfig: additional help text for HMM_MIRROR option

On Fri, Jul 19, 2019 at 01:38:28PM -0700, John Hubbard wrote:
> On 7/19/19 5:04 AM, Pavel Machek wrote:
> > On Fri 2019-07-19 05:00:43, Christoph Hellwig wrote:
> >> On Fri, Jul 19, 2019 at 08:48:53AM -0300, Jason Gunthorpe wrote:
> >>> It is like MMU_NOTIFIERS, if something needs it, then it will select
> >>> it.
> >>>
> >>> Maybe it should just be a hidden kconfig anyhow as there is no reason
> >>> to turn it on without also turning on a using driver.
> >>
> >> We can't just select it due to the odd X86_64 || PPC64 dependency.
> >>
> >> Which also answers Pavels question: you never really need it, as we
> >> can only use it for optional functionality due to that.
> >
> > Okay, just explain it in the help text :-)..
> >
> > Alternatively... you can have WANT_HMM_MIRROR option drivers select,
> > and option HMM_MIRROR which is yes if WANT_HMM_MIRROR && (X86_64 ||
> > PPC64), no?
> >
>
> Yes. This really should be a hidden option that just auto-enables. It's
> not ideal to require people to both *find* HMM_MIRROR, *and* figure out
> that they need it. (I think it's just this way due to the history of how
> HMM got merged--it started off as a kind of experimental sandbox, so
> it had it's own config options, to avoid bothering anything else.)
>
> I'll send out a new patch to just auto-select. The WANT_HMM_MIRROR
> approach seems accurate, given the (X86_64 || PPC64) complication, probably
> after -rc1 is ready (I don't see the ODP code using HMM yet, so that
> must not have been merged yet.)
>
> Longer term, I vaguely recall that there is no strong reason preventing
> HMM from being made to work on other arches, and am hoping that it was
> just done this way to save development time. I don't want to leave it
> this way unless there's a good reason to.

No one has given me a satisfactory answer about the restriction
either.

The only thing this kconfig controls that could possibly be arch
specific is the page walking code in hmm_range_snapshot and
related.

Maybe there is/was some arch entanglement there?

Jason