On Tue, Feb 13, 2024 at 11:51 AM Liam R. Howlett <Liam.Howlett@xxxxxxxxxx> wrote: > > * Lokesh Gidra <lokeshgidra@xxxxxxxxxx> [240213 14:37]: > ... > > > Asking to avoid any more iterations: these functions should call the > > currently defined ones or should replace them. For instance, should I > > do the following: > > > > #ifdef CONFIG_PER_VMA_LOCK > > ... uffd_mfill_lock() > > { > > return find_and_lock_dst_vma(...); > > } > > #else > > ...uffd_mfill_lock() > > { > > return lock_mm_and_find_dst_vma(...); > > } > > #endif > > > > or have the function replace > > find_and_lock_dst_vma()/lock_mm_and_find_dst_vma() ? > > Since the two have the same prototype, then you can replace the function > names directly. > > The other side should take the vma and use vma->vm_mm to get the mm to > unlock the mmap_lock in the !CONFIG_PER_VMA_LOCK. That way those > prototypes also match and can use the same names directly. > > move_pages() requires unlocking two VMAs or one, so pass both VMAs > through and do the check in there. This, unfortunately means that one > of the VMAs will not be used in the !CONFIG_PER_VMA_LOCK case. You > could add an assert to ensure src_vma is locked prior to using dst_vma > to unlock the mmap_lock(), to avoid potential bot emails. > Perfect. Will do that and address the other comments you had on v5 as well. Regarding int vs long for 'err' type, I'm assuming you are ok with my explanation and I should keep long? > Thanks, > Liam