On Wed, Dec 02, 2020 at 08:34:32PM -0500, Pavel Tatashin wrote: > > Either here or perhaps even lower down the call chain when the page is > > captured, similar to how GUP fast would detect it. (how is that done > > anyhow?) > > Ah, thank you for pointing this out. I think I need to address it here: > > https://soleen.com/source/xref/linux/mm/gup.c?r=96e1fac1#94 > > static __maybe_unused struct page *try_grab_compound_head() > if (unlikely(flags & FOLL_LONGTERM) && is_migrate_cma_page(page)) > return NULL; > > I need to change is_migrate_cma_page() to all migratable pages. Will > study, and send an update with this fix. Yes, missing the two flows is a common error :( 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. > 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. Jason
