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@xxxxxxxxxx>
> > Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> > Cc: Balbir Singh <balbirs@xxxxxxxxxxx>
> > Cc: Aneesh Kumar <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
> > Cc: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> > Cc: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
> > ---
> > 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
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
