Messages in this thread Patch in this message |  | | Subject | Re: [PATCH 6/8] hugetlb: add vma based lock for pmd sharing | | From | Miaohe Lin <> | | Date | Sat, 27 Aug 2022 17:30:27 +0800 |
| |
On 2022/8/25 1:57, Mike Kravetz wrote: > Allocate a rw semaphore and hang off vm_private_data for > synchronization use by vmas that could be involved in pmd sharing. Only > add infrastructure for the new lock here. Actual use will be added in > subsequent patch. > > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
<snip>
> +static void hugetlb_vma_lock_free(struct vm_area_struct *vma) > +{ > + /* > + * Only present in sharable vmas. See comment in > + * __unmap_hugepage_range_final about the neeed to check both
s/neeed/need/
> + * VM_SHARED and VM_MAYSHARE in free path
I think there might be some wrong checks around this patch. As above comment said, we need to check both flags, so we should do something like below instead?
if (!(vma->vm_flags & (VM_MAYSHARE | VM_SHARED) == (VM_MAYSHARE | VM_SHARED)))
> + */ > + if (!vma || !(vma->vm_flags & (VM_MAYSHARE | VM_SHARED))) > + return; > + > + if (vma->vm_private_data) { > + kfree(vma->vm_private_data); > + vma->vm_private_data = NULL; > + } > +} > + > +static void hugetlb_vma_lock_alloc(struct vm_area_struct *vma) > +{ > + struct rw_semaphore *vma_sema; > + > + /* Only establish in (flags) sharable vmas */ > + if (!vma || !(vma->vm_flags & VM_MAYSHARE)) > + return; > + > + /* Should never get here with non-NULL vm_private_data */
We can get here with non-NULL vm_private_data when called from hugetlb_vm_op_open during fork?
Also there's one missing change on comment:
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index d0617d64d718..4bc844a1d312 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -863,7 +863,7 @@ __weak unsigned long vma_mmu_pagesize(struct vm_area_struct *vma) * faults in a MAP_PRIVATE mapping. Only the process that called mmap() * is guaranteed to have their future faults succeed. * - * With the exception of reset_vma_resv_huge_pages() which is called at fork(), + * With the exception of hugetlb_dup_vma_private() which is called at fork(), * the reserve counters are updated with the hugetlb_lock held. It is safe * to reset the VMA at fork() time as it is not in use yet and there is no * chance of the global counters getting corrupted as a result of the values.
Otherwise this patch looks good to me. Thanks.
Thanks, Miaohe Lin
|  |