lkml.org 
[lkml]   [2020]   [Dec]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone
    > Looking at this code some more.. How is it even correct?
    >
    > 1633 if (!isolate_lru_page(head)) {
    > 1634 list_add_tail(&head->lru, &cma_page_list);
    >
    > Here we are only running under the read side of the mmap sem so multiple
    > GUPs can be calling that sequence in parallel. I don't see any
    > obvious exclusion that will prevent corruption of head->lru. The first
    > GUP thread to do isolate_lru_page() will ClearPageLRU() and the second
    > GUP thread will be a NOP for isolate_lru_page().
    >
    > They will both race list_add_tail and other list ops. That is not OK.

    Good question. I studied it, and I do not see how this is OK. Worse,
    this race is also exposable as a syscall instead of via driver: two
    move_pages() run simultaneously. Perhaps in other places?

    move_pages()
    kernel_move_pages()
    mmget()
    do_pages_move()
    add_page_for_migratio()
    mmap_read_lock(mm);
    list_add_tail(&head->lru, pagelist); <- Not protected

    >
    > > What I meant is the users of the interface do it incrementally not in
    > > large chunks. For example:
    > >
    > > vfio_pin_pages_remote
    > > vaddr_get_pfn
    > > ret = pin_user_pages_remote(mm, vaddr, 1, flags |
    > > FOLL_LONGTERM, page, NULL, NULL);
    > > 1 -> pin only one pages at a time
    >
    > I don't know why vfio does this, it is why it so ridiculously slow at
    > least.

    Agreed.

    >
    > Jason

    \
     
     \ /
      Last update: 2020-12-03 17:43    [W:3.261 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site