On 2022/7/30 1:41, Mike Kravetz wrote: > On 07/29/22 09:41, Miaohe Lin wrote: >> On 2022/7/29 1:47, Mike Kravetz wrote: >>> On 07/28/22 14:51, Miaohe Lin wrote: >> snip >>>> >>>> Do we need to check &mm->mm_users == 0 here in case the address_space of corresponding process >>>> has exited? In this case, mmdrop will drop the last reference and free the skipped_mm. So we will >>>> use skipped_mm below after it's freed? >>>> >>> >>> Good point! >>> I think we need to wait to drop since we want to hold the read lock. >>> Will update. >>> >>>>> + vma = find_vma(skipped_mm, skipped_vm_start); >>>>> + if (!vma || vma->vm_file->f_mapping != mapping || >>>> >>>> If skipped_vm_start is unmapped and remapped as a anon vma before we taking the mmap_read_lock, >>>> vma->vm_file will be NULL? >>>> >>> >>> IIUC, vma->vm_file will always be set even for an anon vma. The fault >>> code depends on this. See beginning of hugetlb_fault() where we >>> unconditionally do: >>> >>> mapping = vma->vm_file->f_mapping; >> >> What if vma is non-hugetlb anon vma? >> > > Right. The first check after !vma should be for hugetlb as we do not > care if non-hugetlb vmas and want to quit. Something like, > > vma = find_vma(skipped_mm, skipped_vm_start); > if (!vma || !is_vm_hugetlb_page(vma) || > This looks good to me. Thanks.
