[lkml]   [2019]   [Sep]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH v3 3/4] mm: don't expose non-hugetlb page to fast gup prematurely
On 9/26/19 3:20 AM, Kirill A. Shutemov wrote:
> On Wed, Sep 25, 2019 at 04:26:54PM -0600, Yu Zhao wrote:
>> On Wed, Sep 25, 2019 at 10:25:30AM +0200, Peter Zijlstra wrote:
>>> On Tue, Sep 24, 2019 at 05:24:58PM -0600, Yu Zhao wrote:
>>> I'm thinking this patch make stuff rather fragile.. Should we instead
>>> stick the barrier in set_p*d_at() instead? Or rather, make that store a
>>> store-release?
>> I prefer it this way too, but I suspected the majority would be
>> concerned with the performance implications, especially those
>> looping set_pte_at()s in mm/huge_memory.c.
> We can rename current set_pte_at() to __set_pte_at() or something and
> leave it in places where barrier is not needed. The new set_pte_at()( will
> be used in the rest of the places with the barrier inside.

+1, sounds nice. I was unhappy about the wide-ranging changes that would have
to be maintained. So this seems much better.

> BTW, have you looked at other levels of page table hierarchy. Do we have
> the same issue for PMD/PUD/... pages?

Along the lines of "what other memory barriers might be missing for
get_user_pages_fast(), I'm also concerned that the synchronization between
get_user_pages_fast() and freeing the page tables might be technically broken,
due to missing memory barriers on the get_user_pages_fast() side. Details:

gup_fast() disables interrupts, but I think it also needs some sort of
memory barrier(s), in order to prevent reads of the page table (gup_pgd_range,
etc) from speculatively happening before the interrupts are disabled.

Leonardo Bras's recent patchset brought this to my attention. Here, he's
recommending adding atomic counting inc/dec before and after the gup_fast()
irq disable/enable points:

...and that lead to noticing a general lack of barriers there.

John Hubbard

 \ /
  Last update: 2019-09-27 05:27    [W:0.206 / U:0.348 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site