Jason Gunthorpe <jgg@xxxxxxxxxx> writes: > On Thu, Aug 04, 2022 at 01:22:41PM +1000, Alistair Popple wrote: >> When pinning pages with FOLL_LONGTERM check_and_migrate_movable_pages() >> is called to migrate pages out of zones which should not contain any >> longterm pinned pages. >> >> When migration succeeds all pages will have been unpinned so pinning >> needs to be retried. Migration can also fail, in which case the pages >> will also have been unpinned but the operation should not be retried. If >> all pages are in the correct zone nothing will be unpinned and no retry >> is required. >> >> The logic in check_and_migrate_movable_pages() tracks unnecessary state >> and the return codes for each case are difficult to follow. Refactor the >> code to clean this up. No behaviour change is intended. >> >> Signed-off-by: Alistair Popple <apopple@xxxxxxxxxx> >> Cc: "Sierra Guiza, Alejandro (Alex)" <alex.sierra@xxxxxxx> >> Cc: Chaitanya Kulkarni <kch@xxxxxxxxxx> >> Cc: Dan Williams <dan.j.williams@xxxxxxxxx> >> Cc: Felix Kuehling <Felix.Kuehling@xxxxxxx> >> Cc: Jason Gunthorpe <jgg@xxxxxxxxxx> >> Cc: John Hubbard <jhubbard@xxxxxxxxxx> >> Cc: Logan Gunthorpe <logang@xxxxxxxxxxxx> >> Cc: Miaohe Lin <linmiaohe@xxxxxxxxxx> >> Cc: Muchun Song <songmuchun@xxxxxxxxxxxxx> >> Cc: Pasha Tatashin <pasha.tatashin@xxxxxxxxxx> >> Cc: Ralph Campbell <rcampbell@xxxxxxxxxx> >> >> --- >> >> Originally posted as "mm/gup.c: Simplify and fix >> check_and_migrate_movable_pages() return codes"[1]. >> >> Changes from that version: >> >> - Restore the original isolation failure behaviour and don't fail the >> pup. Instead retry indefinitely. >> - Unpin all pages on retry or failure rather than just failure. >> >> Jason - I dropped your Reviewed-by. I had to remove the changes to make >> error handling follow convention as we need to always unpin the pages. >> We also need the list_empty() checks because we may or may not have >> pages in the list if we found coherent pages. So there isn't much I >> could see to simplify, but let me know if you spot some. > > I don't quite understand this, if the point is to loop on the LRU > indefinately, why not just code that? Why do we need to go around the > big loop? I assume by "big loop" you mean calling check_and_migrate_movable_pages() again? We have to do that after the migration anyway. Looping on the LRU indefinitely inside check_and_migrate_movable_pages() doesn't remove that requirement so I'm not convinced it would simplify things much. It could also lead to deadlock because we may already have other pages isolated from the LRU. - Alistair > Jason
