lkml.org 
[lkml]   [2011]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: One (possible) x86 get_user_pages bug
    >>> On 27.01.11 at 14:56, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
    > On Thu, 2011-01-27 at 21:05 +0800, Xiaowei Yang wrote:
    >>
    >> However, from the comments embedded in gup.c, it seems deliberate to
    >> avoid the lock in the fast path. The question is: if so, how to avoid
    >> the above scenario?
    >
    > Something like the below comes to mind... but I must say I haven't fully
    > considered the problem yet..

    That doesn't seem to account for the possible case of the page
    even managing to get allocated again to something else.

    And I think you would need to drop out of gup_pte_range() in
    that case.

    I would think this needs to be get_page_unless_zero()
    followed by re-checking of the page table entry (probably
    not even requiring a second gup_get_pte()); I'm not sure
    yet what the correct action would be for change in only the
    accessed/dirty bits.

    Jan

    > ---
    > arch/x86/mm/gup.c | 9 +++++----
    > 1 files changed, 5 insertions(+), 4 deletions(-)
    >
    > diff --git a/arch/x86/mm/gup.c b/arch/x86/mm/gup.c
    > index dbe34b9..6527933 100644
    > --- a/arch/x86/mm/gup.c
    > +++ b/arch/x86/mm/gup.c
    > @@ -89,10 +89,11 @@ static noinline int gup_pte_range(pmd_t pmd, unsigned
    > long addr,
    > }
    > VM_BUG_ON(!pfn_valid(pte_pfn(pte)));
    > page = pte_page(pte);
    > - get_page(page);
    > - SetPageReferenced(page);
    > - pages[*nr] = page;
    > - (*nr)++;
    > + if (get_page_unless_zero(page)) {
    > + SetPageReferenced(page);
    > + pages[*nr] = page;
    > + (*nr)++;
    > + }
    >
    > } while (ptep++, addr += PAGE_SIZE, addr != end);
    > pte_unmap(ptep - 1);





    \
     
     \ /
      Last update: 2011-01-27 15:33    [W:0.025 / U:0.360 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site