Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1666193 > unrolled thread

[HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64

Started byJérôme Glisse <jglisse@redhat.com>
First post2017-06-14 22:20 +0200
Last post2017-06-15 05:10 +0200
Articles 7 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64 Jérôme Glisse <jglisse@redhat.com> - 2017-06-14 22:20 +0200
    Re: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and  DEVICE_PUBLIC for ppc64 John Hubbard <jhubbard@nvidia.com> - 2017-06-15 01:20 +0200
      Re: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and  DEVICE_PUBLIC for ppc64 Jerome Glisse <jglisse@redhat.com> - 2017-06-15 04:10 +0200
        Re: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and  DEVICE_PUBLIC for ppc64 John Hubbard <jhubbard@nvidia.com> - 2017-06-15 05:20 +0200
    Re: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and  DEVICE_PUBLIC for ppc64 Balbir Singh <bsingharora@gmail.com> - 2017-06-15 04:00 +0200
      Re: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and  DEVICE_PUBLIC for ppc64 Jerome Glisse <jglisse@redhat.com> - 2017-06-15 04:10 +0200
        Re: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and  DEVICE_PUBLIC for ppc64 Balbir Singh <bsingharora@gmail.com> - 2017-06-15 05:10 +0200

#1666193 — [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64

FromJérôme Glisse <jglisse@redhat.com>
Date2017-06-14 22:20 +0200
Subject[HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64
Message-ID<tSmVH-1k9-5@gated-at.bofh.it>
This just simplify kconfig and allow HMM and DEVICE_PUBLIC to be
selected for ppc64 once ZONE_DEVICE is allowed on ppc64 (different
patchset).

Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Cc: Balbir Singh <balbirs@au1.ibm.com>
Cc: Aneesh Kumar <aneesh.kumar@linux.vnet.ibm.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 include/linux/hmm.h |  4 ++--
 mm/Kconfig          | 27 ++++++---------------------
 mm/hmm.c            |  4 ++--
 3 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index f6713b2..720d18c 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -327,7 +327,7 @@ int hmm_vma_fault(struct vm_area_struct *vma,
 #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
 
 
-#if IS_ENABLED(CONFIG_HMM_DEVMEM)
+#if IS_ENABLED(CONFIG_DEVICE_PRIVATE) || IS_ENABLED(CONFIG_DEVICE_PUBLIC)
 struct hmm_devmem;
 
 struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
@@ -456,7 +456,7 @@ struct hmm_device {
  */
 struct hmm_device *hmm_device_new(void *drvdata);
 void hmm_device_put(struct hmm_device *hmm_device);
-#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
+#endif /* CONFIG_DEVICE_PRIVATE || CONFIG_DEVICE_PUBLIC */
 
 
 /* Below are for HMM internal use only! Not to be used by device driver! */
diff --git a/mm/Kconfig b/mm/Kconfig
index ad082b9..7de939a 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -265,7 +265,7 @@ config ARCH_ENABLE_HUGEPAGE_MIGRATION
 config ARCH_HAS_HMM
 	bool
 	default y
-	depends on X86_64
+	depends on X86_64 || PPC64
 	depends on ZONE_DEVICE
 	depends on MMU && 64BIT
 	depends on MEMORY_HOTPLUG
@@ -277,7 +277,7 @@ config HMM
 
 config HMM_MIRROR
 	bool "HMM mirror CPU page table into a device page table"
-	depends on ARCH_HAS_HMM
+	depends on ARCH_HAS_HMM && X86_64
 	select MMU_NOTIFIER
 	select HMM
 	help
@@ -287,15 +287,6 @@ config HMM_MIRROR
 	  page tables (at PAGE_SIZE granularity), and must be able to recover from
 	  the resulting potential page faults.
 
-config HMM_DEVMEM
-	bool "HMM device memory helpers (to leverage ZONE_DEVICE)"
-	depends on ARCH_HAS_HMM
-	select HMM
-	help
-	  HMM devmem is a set of helper routines to leverage the ZONE_DEVICE
-	  feature. This is just to avoid having device drivers to replicating a lot
-	  of boiler plate code.  See Documentation/vm/hmm.txt.
-
 config PHYS_ADDR_T_64BIT
 	def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT
 
@@ -720,11 +711,8 @@ config ZONE_DEVICE
 
 config DEVICE_PRIVATE
 	bool "Unaddressable device memory (GPU memory, ...)"
-	depends on X86_64
-	depends on ZONE_DEVICE
-	depends on MEMORY_HOTPLUG
-	depends on MEMORY_HOTREMOVE
-	depends on SPARSEMEM_VMEMMAP
+	depends on ARCH_HAS_HMM && X86_64
+	select HMM
 
 	help
 	  Allows creation of struct pages to represent unaddressable device
@@ -733,11 +721,8 @@ config DEVICE_PRIVATE
 
 config DEVICE_PUBLIC
 	bool "Unaddressable device memory (GPU memory, ...)"
-	depends on X86_64
-	depends on ZONE_DEVICE
-	depends on MEMORY_HOTPLUG
-	depends on MEMORY_HOTREMOVE
-	depends on SPARSEMEM_VMEMMAP
+	depends on ARCH_HAS_HMM
+	select HMM
 
 	help
 	  Allows creation of struct pages to represent addressable device
diff --git a/mm/hmm.c b/mm/hmm.c
index aed110e..085cc06 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -747,7 +747,7 @@ EXPORT_SYMBOL(hmm_vma_fault);
 #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
 
 
-#if IS_ENABLED(CONFIG_HMM_DEVMEM)
+#if IS_ENABLED(CONFIG_DEVICE_PRIVATE) || IS_ENABLED(CONFIG_DEVICE_PUBLIC)
 struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
 				       unsigned long addr)
 {
@@ -1306,4 +1306,4 @@ static int __init hmm_init(void)
 }
 
 device_initcall(hmm_init);
-#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
+#endif /* CONFIG_DEVICE_PRIVATE || CONFIG_DEVICE_PUBLIC */
-- 
2.9.3

[toc] | [next] | [standalone]

#1666314 — Re: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64

FromJohn Hubbard <jhubbard@nvidia.com>
Date2017-06-15 01:20 +0200
SubjectRe: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64
Message-ID<tSpJU-37v-17@gated-at.bofh.it>
In reply to#1666193
On 06/14/2017 01:11 PM, Jérôme Glisse wrote:
> This just simplify kconfig and allow HMM and DEVICE_PUBLIC to be
> selected for ppc64 once ZONE_DEVICE is allowed on ppc64 (different
> patchset).
> 
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> Cc: Balbir Singh <balbirs@au1.ibm.com>
> Cc: Aneesh Kumar <aneesh.kumar@linux.vnet.ibm.com>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>   include/linux/hmm.h |  4 ++--
>   mm/Kconfig          | 27 ++++++---------------------
>   mm/hmm.c            |  4 ++--
>   3 files changed, 10 insertions(+), 25 deletions(-)
> 
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index f6713b2..720d18c 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -327,7 +327,7 @@ int hmm_vma_fault(struct vm_area_struct *vma,
>   #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
>   
>   
> -#if IS_ENABLED(CONFIG_HMM_DEVMEM)
> +#if IS_ENABLED(CONFIG_DEVICE_PRIVATE) || IS_ENABLED(CONFIG_DEVICE_PUBLIC)
>   struct hmm_devmem;
>   
>   struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
> @@ -456,7 +456,7 @@ struct hmm_device {
>    */
>   struct hmm_device *hmm_device_new(void *drvdata);
>   void hmm_device_put(struct hmm_device *hmm_device);
> -#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
> +#endif /* CONFIG_DEVICE_PRIVATE || CONFIG_DEVICE_PUBLIC */
>   
>   
>   /* Below are for HMM internal use only! Not to be used by device driver! */
> diff --git a/mm/Kconfig b/mm/Kconfig
> index ad082b9..7de939a 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -265,7 +265,7 @@ config ARCH_ENABLE_HUGEPAGE_MIGRATION
>   config ARCH_HAS_HMM
>   	bool
>   	default y
> -	depends on X86_64
> +	depends on X86_64 || PPC64
>   	depends on ZONE_DEVICE
>   	depends on MMU && 64BIT
>   	depends on MEMORY_HOTPLUG
> @@ -277,7 +277,7 @@ config HMM
>   
>   config HMM_MIRROR
>   	bool "HMM mirror CPU page table into a device page table"
> -	depends on ARCH_HAS_HMM
> +	depends on ARCH_HAS_HMM && X86_64
>   	select MMU_NOTIFIER
>   	select HMM
>   	help

Hi Jerome,

There are still some problems with using this configuration. First and foremost, it is still 
possible (and likely, given the complete dissimilarity in naming, and difference in location on the 
screen) to choose HMM_MIRROR, and *not* to choose either DEVICE_PRIVATE or DEVICE_PUBLIC. And then 
we end up with a swath of important page fault handling code being ifdef'd out, and one ends up 
having to investigate why.

As for solutions, at least for the x86 (DEVICE_PRIVATE)case, we could do this:

diff --git a/mm/Kconfig b/mm/Kconfig
index 7de939a29466..f64182d7b956 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -279,6 +279,7 @@ config HMM_MIRROR
         bool "HMM mirror CPU page table into a device page table"
         depends on ARCH_HAS_HMM && X86_64
         select MMU_NOTIFIER
+       select DEVICE_PRIVATE
         select HMM
         help
           Select HMM_MIRROR if you want to mirror range of the CPU page table of a

...and that is better than the other direction (having HMM_MIRROR depend on DEVICE_PRIVATE), because 
in the latter case, HMM_MIRROR will disappear (and it's several lines above) until you select 
DEVICE_PRIVATE. That is hard to work with for the user.

The user will tend to select HMM_MIRROR, but it is *not* obvious that he/she should also select 
DEVICE_PRIVATE. So Kconfig should do it for them.

In fact, I'm not even sure if the DEVICE_PRIVATE and DEVICE_PUBLIC actually need Kconfig protection, 
but if they don't, then life would be easier for whoever is configuring their kernel.


> @@ -287,15 +287,6 @@ config HMM_MIRROR
>   	  page tables (at PAGE_SIZE granularity), and must be able to recover from
>   	  the resulting potential page faults.
>   
> -config HMM_DEVMEM
> -	bool "HMM device memory helpers (to leverage ZONE_DEVICE)"
> -	depends on ARCH_HAS_HMM
> -	select HMM
> -	help
> -	  HMM devmem is a set of helper routines to leverage the ZONE_DEVICE
> -	  feature. This is just to avoid having device drivers to replicating a lot
> -	  of boiler plate code.  See Documentation/vm/hmm.txt.
> -

Yes, probably good to remove HMM_DEVMEM as a separate conig choice.

>   config PHYS_ADDR_T_64BIT
>   	def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT
>   
> @@ -720,11 +711,8 @@ config ZONE_DEVICE
>   
>   config DEVICE_PRIVATE
>   	bool "Unaddressable device memory (GPU memory, ...)"
> -	depends on X86_64
> -	depends on ZONE_DEVICE
> -	depends on MEMORY_HOTPLUG
> -	depends on MEMORY_HOTREMOVE
> -	depends on SPARSEMEM_VMEMMAP
> +	depends on ARCH_HAS_HMM && X86_64
> +	select HMM
>   
>   	help
>   	  Allows creation of struct pages to represent unaddressable device
> @@ -733,11 +721,8 @@ config DEVICE_PRIVATE
>   
>   config DEVICE_PUBLIC
>   	bool "Unaddressable device memory (GPU memory, ...)"

Typo: this is a copy-and-paste from DEVICE_PRIVATE, but the "Unaddressable" part wasn't changed, so 
you'll end up with two identical-looking lines in `make menuconfig`.

Maybe "Directly addressable device memory"? And make the line less identical to DEVICE_PRIVATE?

thanks,
--
John Hubbard
NVIDIA

> -	depends on X86_64
> -	depends on ZONE_DEVICE
> -	depends on MEMORY_HOTPLUG
> -	depends on MEMORY_HOTREMOVE
> -	depends on SPARSEMEM_VMEMMAP
> +	depends on ARCH_HAS_HMM
> +	select HMM
>   
>   	help
>   	  Allows creation of struct pages to represent addressable device
> diff --git a/mm/hmm.c b/mm/hmm.c
> index aed110e..085cc06 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -747,7 +747,7 @@ EXPORT_SYMBOL(hmm_vma_fault);
>   #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
>   
>   
> -#if IS_ENABLED(CONFIG_HMM_DEVMEM)
> +#if IS_ENABLED(CONFIG_DEVICE_PRIVATE) || IS_ENABLED(CONFIG_DEVICE_PUBLIC)
>   struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
>   				       unsigned long addr)
>   {
> @@ -1306,4 +1306,4 @@ static int __init hmm_init(void)
>   }
>   
>   device_initcall(hmm_init);
> -#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
> +#endif /* CONFIG_DEVICE_PRIVATE || CONFIG_DEVICE_PUBLIC */
> -- 
> 2.9.3
>

[toc] | [prev] | [next] | [standalone]

#1666372 — Re: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64

FromJerome Glisse <jglisse@redhat.com>
Date2017-06-15 04:10 +0200
SubjectRe: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64
Message-ID<tSsoq-4ML-5@gated-at.bofh.it>
In reply to#1666314
On Wed, Jun 14, 2017 at 04:10:32PM -0700, John Hubbard wrote:
> On 06/14/2017 01:11 PM, Jérôme Glisse wrote:
> > This just simplify kconfig and allow HMM and DEVICE_PUBLIC to be
> > selected for ppc64 once ZONE_DEVICE is allowed on ppc64 (different
> > patchset).
> > 
> > Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> > Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> > Cc: Balbir Singh <balbirs@au1.ibm.com>
> > Cc: Aneesh Kumar <aneesh.kumar@linux.vnet.ibm.com>
> > Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> >   include/linux/hmm.h |  4 ++--
> >   mm/Kconfig          | 27 ++++++---------------------
> >   mm/hmm.c            |  4 ++--
> >   3 files changed, 10 insertions(+), 25 deletions(-)
> > 
> > diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> > index f6713b2..720d18c 100644
> > --- a/include/linux/hmm.h
> > +++ b/include/linux/hmm.h
> > @@ -327,7 +327,7 @@ int hmm_vma_fault(struct vm_area_struct *vma,
> >   #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
> > -#if IS_ENABLED(CONFIG_HMM_DEVMEM)
> > +#if IS_ENABLED(CONFIG_DEVICE_PRIVATE) || IS_ENABLED(CONFIG_DEVICE_PUBLIC)
> >   struct hmm_devmem;
> >   struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
> > @@ -456,7 +456,7 @@ struct hmm_device {
> >    */
> >   struct hmm_device *hmm_device_new(void *drvdata);
> >   void hmm_device_put(struct hmm_device *hmm_device);
> > -#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
> > +#endif /* CONFIG_DEVICE_PRIVATE || CONFIG_DEVICE_PUBLIC */
> >   /* Below are for HMM internal use only! Not to be used by device driver! */
> > diff --git a/mm/Kconfig b/mm/Kconfig
> > index ad082b9..7de939a 100644
> > --- a/mm/Kconfig
> > +++ b/mm/Kconfig
> > @@ -265,7 +265,7 @@ config ARCH_ENABLE_HUGEPAGE_MIGRATION
> >   config ARCH_HAS_HMM
> >   	bool
> >   	default y
> > -	depends on X86_64
> > +	depends on X86_64 || PPC64
> >   	depends on ZONE_DEVICE
> >   	depends on MMU && 64BIT
> >   	depends on MEMORY_HOTPLUG
> > @@ -277,7 +277,7 @@ config HMM
> >   config HMM_MIRROR
> >   	bool "HMM mirror CPU page table into a device page table"
> > -	depends on ARCH_HAS_HMM
> > +	depends on ARCH_HAS_HMM && X86_64
> >   	select MMU_NOTIFIER
> >   	select HMM
> >   	help
> 
> Hi Jerome,
> 
> There are still some problems with using this configuration. First and
> foremost, it is still possible (and likely, given the complete dissimilarity
> in naming, and difference in location on the screen) to choose HMM_MIRROR,
> and *not* to choose either DEVICE_PRIVATE or DEVICE_PUBLIC. And then we end
> up with a swath of important page fault handling code being ifdef'd out, and
> one ends up having to investigate why.
> 
> As for solutions, at least for the x86 (DEVICE_PRIVATE)case, we could do this:
> 
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 7de939a29466..f64182d7b956 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -279,6 +279,7 @@ config HMM_MIRROR
>         bool "HMM mirror CPU page table into a device page table"
>         depends on ARCH_HAS_HMM && X86_64
>         select MMU_NOTIFIER
> +       select DEVICE_PRIVATE
>         select HMM
>         help
>           Select HMM_MIRROR if you want to mirror range of the CPU page table of a
> 
> ...and that is better than the other direction (having HMM_MIRROR depend on
> DEVICE_PRIVATE), because in the latter case, HMM_MIRROR will disappear (and
> it's several lines above) until you select DEVICE_PRIVATE. That is hard to
> work with for the user.
> 
> The user will tend to select HMM_MIRROR, but it is *not* obvious that he/she
> should also select DEVICE_PRIVATE. So Kconfig should do it for them.
> 
> In fact, I'm not even sure if the DEVICE_PRIVATE and DEVICE_PUBLIC actually
> need Kconfig protection, but if they don't, then life would be easier for
> whoever is configuring their kernel.
> 

We do need Kconfig for DEVICE_PRIVATE and DEVICE_PUBLIC. I can remove HMM_MIRROR
and have HMM mirror code ifdef on DEVICE_PRIVATE.

Cheers,
Jérôme

[toc] | [prev] | [next] | [standalone]

#1666429 — Re: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64

FromJohn Hubbard <jhubbard@nvidia.com>
Date2017-06-15 05:20 +0200
SubjectRe: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64
Message-ID<tStua-5uX-9@gated-at.bofh.it>
In reply to#1666372
On 06/14/2017 07:09 PM, Jerome Glisse wrote:
> On Wed, Jun 14, 2017 at 04:10:32PM -0700, John Hubbard wrote:
>> On 06/14/2017 01:11 PM, Jérôme Glisse wrote:
[...]
>> Hi Jerome,
>>
>> There are still some problems with using this configuration. First and
>> foremost, it is still possible (and likely, given the complete dissimilarity
>> in naming, and difference in location on the screen) to choose HMM_MIRROR,
>> and *not* to choose either DEVICE_PRIVATE or DEVICE_PUBLIC. And then we end
>> up with a swath of important page fault handling code being ifdef'd out, and
>> one ends up having to investigate why.
>>
>> As for solutions, at least for the x86 (DEVICE_PRIVATE)case, we could do this:
>>
>> diff --git a/mm/Kconfig b/mm/Kconfig
>> index 7de939a29466..f64182d7b956 100644
>> --- a/mm/Kconfig
>> +++ b/mm/Kconfig
>> @@ -279,6 +279,7 @@ config HMM_MIRROR
>>          bool "HMM mirror CPU page table into a device page table"
>>          depends on ARCH_HAS_HMM && X86_64
>>          select MMU_NOTIFIER
>> +       select DEVICE_PRIVATE
>>          select HMM
>>          help
>>            Select HMM_MIRROR if you want to mirror range of the CPU page table of a
>>
>> ...and that is better than the other direction (having HMM_MIRROR depend on
>> DEVICE_PRIVATE), because in the latter case, HMM_MIRROR will disappear (and
>> it's several lines above) until you select DEVICE_PRIVATE. That is hard to
>> work with for the user.
>>
>> The user will tend to select HMM_MIRROR, but it is *not* obvious that he/she
>> should also select DEVICE_PRIVATE. So Kconfig should do it for them.
>>
>> In fact, I'm not even sure if the DEVICE_PRIVATE and DEVICE_PUBLIC actually
>> need Kconfig protection, but if they don't, then life would be easier for
>> whoever is configuring their kernel.
>>
> 
> We do need Kconfig for DEVICE_PRIVATE and DEVICE_PUBLIC. I can remove HMM_MIRROR
> and have HMM mirror code ifdef on DEVICE_PRIVATE.
> 
> Cheers,
> Jérôme

That's probably fine.

(I see that you may have missed the rest of my response, but looks like Balbir 
covered it.)

thanks,
john h

[toc] | [prev] | [next] | [standalone]

#1666369 — Re: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64

FromBalbir Singh <bsingharora@gmail.com>
Date2017-06-15 04:00 +0200
SubjectRe: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64
Message-ID<tSseJ-4tL-5@gated-at.bofh.it>
In reply to#1666193
On Wed, 14 Jun 2017 16:11:44 -0400
Jérôme Glisse <jglisse@redhat.com> wrote:

> This just simplify kconfig and allow HMM and DEVICE_PUBLIC to be
> selected for ppc64 once ZONE_DEVICE is allowed on ppc64 (different
> patchset).
> 
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> Cc: Balbir Singh <balbirs@au1.ibm.com>
> Cc: Aneesh Kumar <aneesh.kumar@linux.vnet.ibm.com>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  include/linux/hmm.h |  4 ++--
>  mm/Kconfig          | 27 ++++++---------------------
>  mm/hmm.c            |  4 ++--
>  3 files changed, 10 insertions(+), 25 deletions(-)
> 
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index f6713b2..720d18c 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -327,7 +327,7 @@ int hmm_vma_fault(struct vm_area_struct *vma,
>  #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
>  
>  
> -#if IS_ENABLED(CONFIG_HMM_DEVMEM)
> +#if IS_ENABLED(CONFIG_DEVICE_PRIVATE) || IS_ENABLED(CONFIG_DEVICE_PUBLIC)
>  struct hmm_devmem;
>  
>  struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
> @@ -456,7 +456,7 @@ struct hmm_device {
>   */
>  struct hmm_device *hmm_device_new(void *drvdata);
>  void hmm_device_put(struct hmm_device *hmm_device);
> -#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
> +#endif /* CONFIG_DEVICE_PRIVATE || CONFIG_DEVICE_PUBLIC */
>  
>  
>  /* Below are for HMM internal use only! Not to be used by device driver! */
> diff --git a/mm/Kconfig b/mm/Kconfig
> index ad082b9..7de939a 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -265,7 +265,7 @@ config ARCH_ENABLE_HUGEPAGE_MIGRATION
>  config ARCH_HAS_HMM
>  	bool
>  	default y
> -	depends on X86_64
> +	depends on X86_64 || PPC64

Ideally we want to make this (PPC64 && PPC_BOOK3S)

>  	depends on ZONE_DEVICE
>  	depends on MMU && 64BIT
>  	depends on MEMORY_HOTPLUG
> @@ -277,7 +277,7 @@ config HMM
>  
>  config HMM_MIRROR
>  	bool "HMM mirror CPU page table into a device page table"
> -	depends on ARCH_HAS_HMM
> +	depends on ARCH_HAS_HMM && X86_64

We would need HMM_MIRROR for the generation of hardware that does
not have CDM

>  	select MMU_NOTIFIER
>  	select HMM
>  	help
> @@ -287,15 +287,6 @@ config HMM_MIRROR
>  	  page tables (at PAGE_SIZE granularity), and must be able to recover from
>  	  the resulting potential page faults.
>  
> -config HMM_DEVMEM
> -	bool "HMM device memory helpers (to leverage ZONE_DEVICE)"
> -	depends on ARCH_HAS_HMM
> -	select HMM
> -	help
> -	  HMM devmem is a set of helper routines to leverage the ZONE_DEVICE
> -	  feature. This is just to avoid having device drivers to replicating a lot
> -	  of boiler plate code.  See Documentation/vm/hmm.txt.
> -
>  config PHYS_ADDR_T_64BIT
>  	def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT
>  
> @@ -720,11 +711,8 @@ config ZONE_DEVICE
>  
>  config DEVICE_PRIVATE
>  	bool "Unaddressable device memory (GPU memory, ...)"
> -	depends on X86_64
> -	depends on ZONE_DEVICE
> -	depends on MEMORY_HOTPLUG
> -	depends on MEMORY_HOTREMOVE
> -	depends on SPARSEMEM_VMEMMAP
> +	depends on ARCH_HAS_HMM && X86_64

Same as above

> +	select HMM
>  
>  	help
>  	  Allows creation of struct pages to represent unaddressable device
> @@ -733,11 +721,8 @@ config DEVICE_PRIVATE
>  
>  config DEVICE_PUBLIC
>  	bool "Unaddressable device memory (GPU memory, ...)"

The unaddressable is a typo from above.

> -	depends on X86_64
> -	depends on ZONE_DEVICE
> -	depends on MEMORY_HOTPLUG
> -	depends on MEMORY_HOTREMOVE
> -	depends on SPARSEMEM_VMEMMAP
> +	depends on ARCH_HAS_HMM
> +	select HMM
>  

Balbir Singh.

[toc] | [prev] | [next] | [standalone]

#1666375 — Re: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64

FromJerome Glisse <jglisse@redhat.com>
Date2017-06-15 04:10 +0200
SubjectRe: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64
Message-ID<tSsoq-4ML-9@gated-at.bofh.it>
In reply to#1666369
On Thu, Jun 15, 2017 at 11:46:11AM +1000, Balbir Singh wrote:
> On Wed, 14 Jun 2017 16:11:44 -0400
> Jérôme Glisse <jglisse@redhat.com> wrote:
> 
> > This just simplify kconfig and allow HMM and DEVICE_PUBLIC to be
> > selected for ppc64 once ZONE_DEVICE is allowed on ppc64 (different
> > patchset).
> > 
> > Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> > Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> > Cc: Balbir Singh <balbirs@au1.ibm.com>
> > Cc: Aneesh Kumar <aneesh.kumar@linux.vnet.ibm.com>
> > Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> >  include/linux/hmm.h |  4 ++--
> >  mm/Kconfig          | 27 ++++++---------------------
> >  mm/hmm.c            |  4 ++--
> >  3 files changed, 10 insertions(+), 25 deletions(-)
> > 
> > diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> > index f6713b2..720d18c 100644
> > --- a/include/linux/hmm.h
> > +++ b/include/linux/hmm.h
> > @@ -327,7 +327,7 @@ int hmm_vma_fault(struct vm_area_struct *vma,
> >  #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
> >  
> >  
> > -#if IS_ENABLED(CONFIG_HMM_DEVMEM)
> > +#if IS_ENABLED(CONFIG_DEVICE_PRIVATE) || IS_ENABLED(CONFIG_DEVICE_PUBLIC)
> >  struct hmm_devmem;
> >  
> >  struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
> > @@ -456,7 +456,7 @@ struct hmm_device {
> >   */
> >  struct hmm_device *hmm_device_new(void *drvdata);
> >  void hmm_device_put(struct hmm_device *hmm_device);
> > -#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
> > +#endif /* CONFIG_DEVICE_PRIVATE || CONFIG_DEVICE_PUBLIC */
> >  
> >  
> >  /* Below are for HMM internal use only! Not to be used by device driver! */
> > diff --git a/mm/Kconfig b/mm/Kconfig
> > index ad082b9..7de939a 100644
> > --- a/mm/Kconfig
> > +++ b/mm/Kconfig
> > @@ -265,7 +265,7 @@ config ARCH_ENABLE_HUGEPAGE_MIGRATION
> >  config ARCH_HAS_HMM
> >  	bool
> >  	default y
> > -	depends on X86_64
> > +	depends on X86_64 || PPC64
> 
> Ideally we want to make this (PPC64 && PPC_BOOK3S)

BOOK3S really ? :)

> 
> >  	depends on ZONE_DEVICE
> >  	depends on MMU && 64BIT
> >  	depends on MEMORY_HOTPLUG
> > @@ -277,7 +277,7 @@ config HMM
> >  
> >  config HMM_MIRROR
> >  	bool "HMM mirror CPU page table into a device page table"
> > -	depends on ARCH_HAS_HMM
> > +	depends on ARCH_HAS_HMM && X86_64
> 
> We would need HMM_MIRROR for the generation of hardware that does
> not have CDM

That would require could change to mirror code mostly ppc is missing
something like pmd_index() iirc. So best to tackle that as separate
patchset.

> 
> >  	select MMU_NOTIFIER
> >  	select HMM
> >  	help
> > @@ -287,15 +287,6 @@ config HMM_MIRROR
> >  	  page tables (at PAGE_SIZE granularity), and must be able to recover from
> >  	  the resulting potential page faults.
> >  
> > -config HMM_DEVMEM
> > -	bool "HMM device memory helpers (to leverage ZONE_DEVICE)"
> > -	depends on ARCH_HAS_HMM
> > -	select HMM
> > -	help
> > -	  HMM devmem is a set of helper routines to leverage the ZONE_DEVICE
> > -	  feature. This is just to avoid having device drivers to replicating a lot
> > -	  of boiler plate code.  See Documentation/vm/hmm.txt.
> > -
> >  config PHYS_ADDR_T_64BIT
> >  	def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT
> >  
> > @@ -720,11 +711,8 @@ config ZONE_DEVICE
> >  
> >  config DEVICE_PRIVATE
> >  	bool "Unaddressable device memory (GPU memory, ...)"
> > -	depends on X86_64
> > -	depends on ZONE_DEVICE
> > -	depends on MEMORY_HOTPLUG
> > -	depends on MEMORY_HOTREMOVE
> > -	depends on SPARSEMEM_VMEMMAP
> > +	depends on ARCH_HAS_HMM && X86_64
> 
> Same as above
> 
> > +	select HMM
> >  
> >  	help
> >  	  Allows creation of struct pages to represent unaddressable device
> > @@ -733,11 +721,8 @@ config DEVICE_PRIVATE
> >  
> >  config DEVICE_PUBLIC
> >  	bool "Unaddressable device memory (GPU memory, ...)"
> 
> The unaddressable is a typo from above.

Yup cut and paste thank for catching that.

Jérôme

[toc] | [prev] | [next] | [standalone]

#1666403 — Re: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64

FromBalbir Singh <bsingharora@gmail.com>
Date2017-06-15 05:10 +0200
SubjectRe: [HMM-CDM 5/5] mm/hmm: simplify kconfig and enable HMM and DEVICE_PUBLIC for ppc64
Message-ID<tStkt-5rK-1@gated-at.bofh.it>
In reply to#1666375
On Wed, 14 Jun 2017 22:07:09 -0400
Jerome Glisse <jglisse@redhat.com> wrote:

> On Thu, Jun 15, 2017 at 11:46:11AM +1000, Balbir Singh wrote:
> > On Wed, 14 Jun 2017 16:11:44 -0400
> > Jérôme Glisse <jglisse@redhat.com> wrote:
> >   
> > > This just simplify kconfig and allow HMM and DEVICE_PUBLIC to be
> > > selected for ppc64 once ZONE_DEVICE is allowed on ppc64 (different
> > > patchset).
> > > 
> > > Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> > > Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> > > Cc: Balbir Singh <balbirs@au1.ibm.com>
> > > Cc: Aneesh Kumar <aneesh.kumar@linux.vnet.ibm.com>
> > > Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > ---
> > >  include/linux/hmm.h |  4 ++--
> > >  mm/Kconfig          | 27 ++++++---------------------
> > >  mm/hmm.c            |  4 ++--
> > >  3 files changed, 10 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> > > index f6713b2..720d18c 100644
> > > --- a/include/linux/hmm.h
> > > +++ b/include/linux/hmm.h
> > > @@ -327,7 +327,7 @@ int hmm_vma_fault(struct vm_area_struct *vma,
> > >  #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
> > >  
> > >  
> > > -#if IS_ENABLED(CONFIG_HMM_DEVMEM)
> > > +#if IS_ENABLED(CONFIG_DEVICE_PRIVATE) || IS_ENABLED(CONFIG_DEVICE_PUBLIC)
> > >  struct hmm_devmem;
> > >  
> > >  struct page *hmm_vma_alloc_locked_page(struct vm_area_struct *vma,
> > > @@ -456,7 +456,7 @@ struct hmm_device {
> > >   */
> > >  struct hmm_device *hmm_device_new(void *drvdata);
> > >  void hmm_device_put(struct hmm_device *hmm_device);
> > > -#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
> > > +#endif /* CONFIG_DEVICE_PRIVATE || CONFIG_DEVICE_PUBLIC */
> > >  
> > >  
> > >  /* Below are for HMM internal use only! Not to be used by device driver! */
> > > diff --git a/mm/Kconfig b/mm/Kconfig
> > > index ad082b9..7de939a 100644
> > > --- a/mm/Kconfig
> > > +++ b/mm/Kconfig
> > > @@ -265,7 +265,7 @@ config ARCH_ENABLE_HUGEPAGE_MIGRATION
> > >  config ARCH_HAS_HMM
> > >  	bool
> > >  	default y
> > > -	depends on X86_64
> > > +	depends on X86_64 || PPC64  
> > 
> > Ideally we want to make this (PPC64 && PPC_BOOK3S)  
> 
> BOOK3S really ? :)

Sorry what's wrong with that? I am not sure we want it for BOOK3E 64 bit
Or may be would just make ARCH_HAS_HMM a config set by the ARCH,
like ARCH_HAS_SET_MEMORY

> 
> >   
> > >  	depends on ZONE_DEVICE
> > >  	depends on MMU && 64BIT
> > >  	depends on MEMORY_HOTPLUG
> > > @@ -277,7 +277,7 @@ config HMM
> > >  
> > >  config HMM_MIRROR
> > >  	bool "HMM mirror CPU page table into a device page table"
> > > -	depends on ARCH_HAS_HMM
> > > +	depends on ARCH_HAS_HMM && X86_64  
> > 
> > We would need HMM_MIRROR for the generation of hardware that does
> > not have CDM  
> 
> That would require could change to mirror code mostly ppc is missing
> something like pmd_index() iirc. So best to tackle that as separate
> patchset.
> 

We do have pmd_index() and I think Reza's got the whole hmm to compile
on powerpc. I am OK tackling it later. Do you suspect there is a compile time
dependency? I got HMM_MIRROR to compile for me as well

> >   
> > >  	select MMU_NOTIFIER
> > >  	select HMM
> > >  	help
> > > @@ -287,15 +287,6 @@ config HMM_MIRROR
> > >  	  page tables (at PAGE_SIZE granularity), and must be able to recover from
> > >  	  the resulting potential page faults.
> > >  
> > > -config HMM_DEVMEM
> > > -	bool "HMM device memory helpers (to leverage ZONE_DEVICE)"
> > > -	depends on ARCH_HAS_HMM
> > > -	select HMM
> > > -	help
> > > -	  HMM devmem is a set of helper routines to leverage the ZONE_DEVICE
> > > -	  feature. This is just to avoid having device drivers to replicating a lot
> > > -	  of boiler plate code.  See Documentation/vm/hmm.txt.
> > > -
> > >  config PHYS_ADDR_T_64BIT
> > >  	def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT
> > >  
> > > @@ -720,11 +711,8 @@ config ZONE_DEVICE
> > >  
> > >  config DEVICE_PRIVATE
> > >  	bool "Unaddressable device memory (GPU memory, ...)"
> > > -	depends on X86_64
> > > -	depends on ZONE_DEVICE
> > > -	depends on MEMORY_HOTPLUG
> > > -	depends on MEMORY_HOTREMOVE
> > > -	depends on SPARSEMEM_VMEMMAP
> > > +	depends on ARCH_HAS_HMM && X86_64  
> > 
> > Same as above
> >   
> > > +	select HMM
> > >  
> > >  	help
> > >  	  Allows creation of struct pages to represent unaddressable device
> > > @@ -733,11 +721,8 @@ config DEVICE_PRIVATE
> > >  
> > >  config DEVICE_PUBLIC
> > >  	bool "Unaddressable device memory (GPU memory, ...)"  
> > 
> > The unaddressable is a typo from above.  
> 
> Yup cut and paste thank for catching that.

Your welcome
Balbir Singh.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web