Re: [akpm-mm:mm-new] BUILD SUCCESS WITH UNVERIFIED WARNING 34cacd0ac7c10dc9e34e1b9f28a2b78d335cb487

Stanislav Kinsburskii <[email protected]> Sun, 26 Jul 2026 11:41:26 -0700
Newsgroups org.kernel.vger.mm-commits,org.kvack.linux-mm
Message-ID <amZU1hH_9IOeO4G8@skinsburskii>
On Sat, Jul 25, 2026 at 07:31:55PM -0700, Andrew Morton wrote:
> 
> Thanks.  A few head-scratchers for hmm people, please:
> 
> On Sat, 25 Jul 2026 15:11:38 +0800 kernel test robot <[email protected]> wrote:
> 
> > tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-new
> > branch HEAD: 34cacd0ac7c10dc9e34e1b9f28a2b78d335cb487  mm/swap, PM: hibernate: atomically replace hibernation pin
> > 
> > Unverified Warning (likely false positive, kindly check if interested):
> > 
> >     https://lore.kernel.org/oe-kbuild/[email protected]
> >     https://lore.kernel.org/oe-kbuild/[email protected]
> > 
> >     mm/hmm.c:673 hmm_do_fault() error: we previously assumed 'hmm_vma_walk->locked' could be null (see line 654)
> >     mm/vmpressure.c:123 vmpressure() warn: check likely/unlikely parentheses
> > 
> > Warning ids grouped by kconfigs:
> > 
> > recent_errors
> > |-- microblaze-randconfig-r072-20260725
> > |   `-- mm-hmm.c-hmm_do_fault()-error:we-previously-assumed-hmm_vma_walk-locked-could-be-null-(see-line-)
> 
> With this microblaze config on current mainline I hit
> 
> mm/hmm.c:804:25: error: implicit declaration of function 'mmu_interval_read_begin'; did you mean 'mmu_interval_check_retry'? [-Wimplicit-function-declaration]
>   804 |                         mmu_interval_read_begin(range->notifier);
> 
> And after adding the hmm changes from mm-unstable I also hit
> 
> mm/hmm.c:805:25: error: implicit declaration of function 'mmu_interval_read_begin'; did you mean 'mmu_interval_check_retry'? [-Wimplicit-function-declaration]
>   805 |                         mmu_interval_read_begin(range->notifier);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~
>       |                         mmu_interval_check_retry
> 
> Both of which can be fixed with
> 
> --- a/include/linux/mmu_notifier.h~a
> +++ a/include/linux/mmu_notifier.h
> @@ -576,6 +576,19 @@ static inline void _mmu_notifier_range_i
>  	_mmu_notifier_range_init(range, start, end)
>  
>  static inline bool
> +mmu_interval_check_retry(struct mmu_interval_notifier *interval_sub,
> +			 unsigned long seq)
> +{
> +	return true;
> +}
> +
> +static inline unsigned long
> +mmu_interval_read_begin(struct mmu_interval_notifier *interval_sub)
> +{
> +	return 0;
> +}
> +
> +static inline bool
>  mmu_notifier_range_blockable(const struct mmu_notifier_range *range)
>  {
>  	return true;
> 
> But I do wonder whether we should be compiling hmm at all if
> CONFIG_MMU_NOTIFIER=n?
> 

Documentation/mm/hmm.rst explicitly states:

"Address space mirroring's main objective is to allow duplication of a range of
CPU page table into a device page table; HMM helps keep both synchronized. A
device driver that wants to mirror a process address space must start with the
registration of a mmu_interval_notifier"

I think making CONFIG_HMM_MIRROR dependent on the CONFIG_MMU_NOTIFIER is the right thing to do.

> 
> And your hmm_do_fault() warning seems legit:
> 
> 	if (hmm_vma_walk->locked)
> 		fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
> 
> ...
> 			*hmm_vma_walk->locked = false;
> 
> and the blame seems to lie with Stanislav's "mm/hmm: add
> hmm_range_fault_unlocked_timeout() for mmap lock-drop support".
> 

Well, this is the same behavior as in fixup_user_fault().

The fault handler is not supposed to return either VM_FAULT_COMPLETED or
VM_FAULT_RETRY unless FAULT_FLAG_ALLOW_RETRY is set, and that flag is
set only if hmm_vma_walk->locked is not NULL.

In other words, only buggy fault handlers can trigger this NULL pointer
dereference, and I thought we had agreed that we do not try to support
those.

Let me know if you think otherwise and I'll send an update.

Thanks, Stanislav

>