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 >