Re: [PATCH v2 01/11] mm/hmm: select mmu notifier when selecting HMM

[John Hubbard <jhubbard@nvidia.com>]

On 3/25/19 7:40 AM, jglisse@redhat.com wrote:
> From: Jérôme Glisse <jglisse@redhat.com>
> 
> To avoid random config build issue, select mmu notifier when HMM is
> selected. In any cases when HMM get selected it will be by users that
> will also wants the mmu notifier.
> 
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> Acked-by: Balbir Singh <bsingharora@gmail.com>
> Cc: Ralph Campbell <rcampbell@nvidia.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: John Hubbard <jhubbard@nvidia.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> ---
>  mm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 25c71eb8a7db..0d2944278d80 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -694,6 +694,7 @@ config DEV_PAGEMAP_OPS
>  
>  config HMM
>  	bool
> +	select MMU_NOTIFIER
>  	select MIGRATE_VMA_HELPER
>  
>  config HMM_MIRROR
> 

Yes, this is a good move, given that MMU notifiers are completely,
indispensably part of the HMM design and implementation.

The alternative would also work, but it's not quite as good. I'm
listing it in order to forestall any debate: 

  config HMM
  	bool
 +	depends on MMU_NOTIFIER
  	select MIGRATE_VMA_HELPER

...and "depends on" versus "select" is always a subtle question. But in
this case, I'd say that if someone wants HMM, there's no advantage in
making them know that they must first ensure MMU_NOTIFIER is enabled.
After poking around a bit I don't see any obvious downsides either.

However, given that you're making this change, in order to avoid odd
redundancy, you should also do this:

diff --git a/mm/Kconfig b/mm/Kconfig
index 0d2944278d80..2e6d24d783f7 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -700,7 +700,6 @@ config HMM
 config HMM_MIRROR
        bool "HMM mirror CPU page table into a device page table"
        depends on ARCH_HAS_HMM
-       select MMU_NOTIFIER
        select HMM
        help
          Select HMM_MIRROR if you want to mirror range of the CPU page table of a


thanks,
-- 
John Hubbard
NVIDIA