On 12/7/22 14:43, Peter Xu wrote:
Note that here migration_entry_wait_huge() will release it.
Sorry it's definitely not as straightforward, but this is also something I
didn't come up with a better solution, because we need the vma lock to
protect the spinlock, which is used in deep code path of the migration
code.
That's also why I added a rich comment above, and there's "The vma lock
will be released there" which is just for that.
Yes, OK,
Reviewed-by: John Hubbard <jhubbard@xxxxxxxxxx>
...and here is some fancy documentation polishing (incremental on top of this
specific patch) if you would like to fold it in, optional but it makes me
happier:
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 49f73677a418..e3bbd4869f68 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5809,6 +5809,10 @@ u32 hugetlb_fault_mutex_hash(struct address_space *mapping, pgoff_t idx)
}
#endif
+/*
+ * There are a few special cases in which this function returns while still
+ * holding locks. Those are noted inline.
+ */
vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
unsigned long address, unsigned int flags)
{
@@ -5851,8 +5855,8 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
/* PTE markers should be handled the same way as none pte */
if (huge_pte_none_mostly(entry))
/*
- * hugetlb_no_page will drop vma lock and hugetlb fault
- * mutex internally, which make us return immediately.
+ * hugetlb_no_page() will release both the vma lock and the
+ * hugetlb fault mutex, so just return directly from that:
*/
return hugetlb_no_page(mm, vma, mapping, idx, address, ptep,
entry, flags);
@@ -5869,10 +5873,11 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
if (!pte_present(entry)) {
if (unlikely(is_hugetlb_entry_migration(entry))) {
/*
- * Release fault lock first because the vma lock is
- * needed to guard the huge_pte_lockptr() later in
- * migration_entry_wait_huge(). The vma lock will
- * be released there.
+ * Release the hugetlb fault lock now, but retain the
+ * vma lock, because it is needed to guard the
+ * huge_pte_lockptr() later in
+ * migration_entry_wait_huge(). The vma lock will be
+ * released there.
*/
mutex_unlock(&hugetlb_fault_mutex_table[hash]);
migration_entry_wait_huge(vma, ptep);
diff --git a/mm/migrate.c b/mm/migrate.c
index d14f1f3ab073..a31df628b938 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -333,16 +333,18 @@ void migration_entry_wait(struct mm_struct *mm, pmd_t *pmd,
}
#ifdef CONFIG_HUGETLB_PAGE
+
+/*
+ * The vma read lock must be held upon entry. Holding that lock prevents either
+ * the pte or the ptl from being freed.
+ *
+ * This function will release the vma lock before returning.
+ */
void __migration_entry_wait_huge(struct vm_area_struct *vma,
pte_t *ptep, spinlock_t *ptl)
{
pte_t pte;
- /*
- * The vma read lock must be taken, which will be released before
- * the function returns. It makes sure the pgtable page (along
- * with its spin lock) not be freed in parallel.
- */
hugetlb_vma_assert_locked(vma);
spin_lock(ptl);
thanks,
--
John Hubbard
NVIDIA
