| Message ID | YLX8uYN01JmfLnlK@mwanda (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm: thp: fix a double unlock bug | expand |
On Tue, Jun 01, 2021 at 12:24:09PM +0300, Dan Carpenter wrote: > We're supposed to be holding the "vmf->ptl" spin_lock when we goto > out_map. The lock is dropped after if finishes cleaning up. > > Fixes: 9aff7b33c74a ("mm: thp: refactor NUMA fault handling") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Ouch. Acked-by: Mel Gorman <mgorman@suse.de> However, that git commit is not stable. Instead of Fixes: I would suggest renaming the patch to "mm: thp: refactor NUMA fault handling -fix" and replacing Fixes with "This patch is a fix to the mmotm patch mm-thp-refactor-numa-fault-handling.patch". Andrew usually slots that into the correct place in his quilt series and collapses the fixes before sending to Linus which works better with bisection.
On Tue, Jun 01, 2021 at 11:08:49AM +0100, Mel Gorman wrote: > On Tue, Jun 01, 2021 at 12:24:09PM +0300, Dan Carpenter wrote: > > We're supposed to be holding the "vmf->ptl" spin_lock when we goto > > out_map. The lock is dropped after if finishes cleaning up. > > > > Fixes: 9aff7b33c74a ("mm: thp: refactor NUMA fault handling") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > Ouch. > > Acked-by: Mel Gorman <mgorman@suse.de> > > However, that git commit is not stable. Instead of Fixes: I would > suggest renaming the patch to "mm: thp: refactor NUMA fault handling > -fix" and replacing Fixes with "This patch is a fix to the mmotm patch > mm-thp-refactor-numa-fault-handling.patch". Andrew usually slots that > into the correct place in his quilt series and collapses the fixes before > sending to Linus which works better with bisection. I know that these normally get folded in, but I assumed that Andrew would want the Fixes tag so that he could fold them in automatically using a mutt alias. #OneClickShopping regards, dan carpenter
On Tue, Jun 1, 2021 at 2:24 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > We're supposed to be holding the "vmf->ptl" spin_lock when we goto > out_map. The lock is dropped after if finishes cleaning up. > > Fixes: 9aff7b33c74a ("mm: thp: refactor NUMA fault handling") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Thanks for catching this. Acked-by: Yang Shi <shy828301@gmail.com> > --- > mm/huge_memory.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index e353bbc6cee3..caa0148f15bb 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -1465,10 +1465,8 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf) > > pmd = pmd_modify(oldpmd, vma->vm_page_prot); > page = vm_normal_page_pmd(vma, haddr, pmd); > - if (!page) { > - spin_unlock(vmf->ptl); > + if (!page) > goto out_map; > - } > > /* See similar comment in do_numa_page for explanation */ > if (!was_writable) > -- > 2.30.2 >
diff --git a/mm/huge_memory.c b/mm/huge_memory.c index e353bbc6cee3..caa0148f15bb 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -1465,10 +1465,8 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf) pmd = pmd_modify(oldpmd, vma->vm_page_prot); page = vm_normal_page_pmd(vma, haddr, pmd); - if (!page) { - spin_unlock(vmf->ptl); + if (!page) goto out_map; - } /* See similar comment in do_numa_page for explanation */ if (!was_writable)
We're supposed to be holding the "vmf->ptl" spin_lock when we goto out_map. The lock is dropped after if finishes cleaning up. Fixes: 9aff7b33c74a ("mm: thp: refactor NUMA fault handling") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- mm/huge_memory.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)