Messages in this thread | | | Date | Sat, 14 Jan 2012 13:27:19 +0800 | Subject | Re: [PATCH] mm: hugetlb: undo change to page mapcount in fault handler | From | Hillf Danton <> |
| |
On Sat, Jan 14, 2012 at 7:39 AM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Wed, 11 Jan 2012 20:06:30 +0800 > Hillf Danton <dhillf@gmail.com> wrote: > >> On Thu, Jan 5, 2012 at 7:16 AM, Andrew Morton <akpm@linux-foundation.org> wrote: >> > On Fri, 23 Dec 2011 21:00:41 +0800 >> > Hillf Danton <dhillf@gmail.com> wrote: >> > >> >> Page mapcount should be updated only if we are sure that the page ends >> >> up in the page table otherwise we would leak if we couldn't COW due to >> >> reservations or if idx is out of bounds. >> > >> > It would be much nicer if we could run vma_needs_reservation() before >> > even looking up or allocating the page. >> > >> > And afaict the interface is set up to do that: you run >> > vma_needs_reservation() before allocating the page and then >> > vma_commit_reservation() afterwards. >> > >> > But hugetlb_no_page() and hugetlb_fault() appear to have forgotten to >> > run vma_commit_reservation() altogether. __Why isn't this as busted as >> > it appears to be? >> >> Hi Andrew >> >> IIUC the two operations, vma_{needs, commit}_reservation, are folded in >> alloc_huge_page(), need to break the pair? > > Looking at it again, it appears that the vma_needs_reservation() calls > are used to predict whether a subsequent COW attempt is going to fail. > > If that's correct then things aren't as bad as I first thought. > However I suspect the code in hugetlb_no_page() is a bit racy: the > vma_needs_reservation() call should happen after we've taken > page_table_lock. As things stand, another thread could sneak in there > and steal the reservation which this thread thought was safe. > > What do you think? >
Hi Andrew
The case of no page, in the fault path, is handled after acquiring hugetlb_instantiation_mutex, and on ohter hand, kmalloc is called if new region required, so no race to check reservation needed but after spinning page_table_lock.
Thanks Hillf -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |