Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-08 Thread Jason Gunthorpe
On Mon, Dec 07, 2020 at 09:48:48PM -0500, Daniel Jordan wrote: > Jason Gunthorpe writes: > > On Fri, Dec 04, 2020 at 03:05:46PM -0500, Daniel Jordan wrote: > >> Well Alex can correct me, but I went digging and a comment from the > >> first type1 vfio commit says the iommu API didn't promise to

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-07 Thread Daniel Jordan
Jason Gunthorpe writes: > On Fri, Dec 04, 2020 at 03:05:46PM -0500, Daniel Jordan wrote: >> Well Alex can correct me, but I went digging and a comment from the >> first type1 vfio commit says the iommu API didn't promise to unmap >> subpages of previous mappings, so doing page at a time gave

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-07 Thread Daniel Jordan
Pavel Tatashin writes: > On Fri, Dec 4, 2020 at 3:06 PM Daniel Jordan > wrote: >> >> Jason Gunthorpe writes: >> >> > On Wed, Dec 02, 2020 at 08:34:32PM -0500, Pavel Tatashin wrote: >> >> What I meant is the users of the interface do it incrementally not in >> >> large chunks. For example: >>

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-06 Thread Joonsoo Kim
On Fri, Dec 04, 2020 at 12:43:29PM -0500, Pavel Tatashin wrote: > On Thu, Dec 3, 2020 at 11:14 PM Joonsoo Kim wrote: > > > > On Wed, Dec 02, 2020 at 12:23:30AM -0500, Pavel Tatashin wrote: > > > We do not allocate pin pages in ZONE_MOVABLE, but if pages were already > > > allocated before pinning

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-04 Thread Jason Gunthorpe
On Fri, Dec 04, 2020 at 03:05:46PM -0500, Daniel Jordan wrote: > Jason Gunthorpe writes: > > > On Wed, Dec 02, 2020 at 08:34:32PM -0500, Pavel Tatashin wrote: > >> What I meant is the users of the interface do it incrementally not in > >> large chunks. For example: > >> > >>

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-04 Thread Pavel Tatashin
On Fri, Dec 4, 2020 at 3:06 PM Daniel Jordan wrote: > > Jason Gunthorpe writes: > > > On Wed, Dec 02, 2020 at 08:34:32PM -0500, Pavel Tatashin wrote: > >> What I meant is the users of the interface do it incrementally not in > >> large chunks. For example: > >> > >> vfio_pin_pages_remote > >>

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-04 Thread Daniel Jordan
Jason Gunthorpe writes: > On Wed, Dec 02, 2020 at 08:34:32PM -0500, Pavel Tatashin wrote: >> 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,

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-04 Thread Pavel Tatashin
On Thu, Dec 3, 2020 at 11:14 PM Joonsoo Kim wrote: > > On Wed, Dec 02, 2020 at 12:23:30AM -0500, Pavel Tatashin wrote: > > We do not allocate pin pages in ZONE_MOVABLE, but if pages were already > > allocated before pinning they need to migrated to a different zone. > > Currently, we migrate

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-04 Thread Jason Gunthorpe
On Fri, Dec 04, 2020 at 11:24:56AM -0500, Pavel Tatashin wrote: > On Thu, Dec 3, 2020 at 2:36 PM Jason Gunthorpe wrote: > > > > On Thu, Dec 03, 2020 at 02:15:36PM -0500, Pavel Tatashin wrote: > > > > > I studied some more, and I think this is not a race: > > > list_add_tail(>lru, _page_list) is

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-04 Thread Pavel Tatashin
On Thu, Dec 3, 2020 at 2:36 PM Jason Gunthorpe wrote: > > On Thu, Dec 03, 2020 at 02:15:36PM -0500, Pavel Tatashin wrote: > > > I studied some more, and I think this is not a race: > > list_add_tail(>lru, _page_list) is called only when > > isolate_lru_page(head) succeeds. > >

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-03 Thread Joonsoo Kim
On Wed, Dec 02, 2020 at 12:23:30AM -0500, Pavel Tatashin wrote: > We do not allocate pin pages in ZONE_MOVABLE, but if pages were already > allocated before pinning they need to migrated to a different zone. > Currently, we migrate movable CMA pages only. Generalize the function > that migrates

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-03 Thread Jason Gunthorpe
On Thu, Dec 03, 2020 at 02:15:36PM -0500, Pavel Tatashin wrote: > I studied some more, and I think this is not a race: > list_add_tail(>lru, _page_list) is called only when > isolate_lru_page(head) succeeds. > isolate_lru_page(head) succeeds only when PageLRU(head) is true. > However, in this

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-03 Thread Pavel Tatashin
> > > > Looking at this code some more.. How is it even correct? > > > > > > > > 1633if (!isolate_lru_page(head)) { > > > > 1634list_add_tail(>lru, > > > > _page_list); > > > > > > > > Here we are only running under the read side of

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-03 Thread Pavel Tatashin
On Thu, Dec 3, 2020 at 11:59 AM Jason Gunthorpe wrote: > > On Thu, Dec 03, 2020 at 11:40:15AM -0500, Pavel Tatashin wrote: > > > Looking at this code some more.. How is it even correct? > > > > > > 1633if (!isolate_lru_page(head)) { > > > 1634

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-03 Thread Jason Gunthorpe
On Thu, Dec 03, 2020 at 11:40:15AM -0500, Pavel Tatashin wrote: > > Looking at this code some more.. How is it even correct? > > > > 1633if (!isolate_lru_page(head)) { > > 1634list_add_tail(>lru, > > _page_list); > > > > Here we are

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-03 Thread Pavel Tatashin
> Looking at this code some more.. How is it even correct? > > 1633if (!isolate_lru_page(head)) { > 1634list_add_tail(>lru, > _page_list); > > Here we are only running under the read side of the mmap sem so multiple > GUPs can be

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-03 Thread Pavel Tatashin
> I like the cleanup so far, even at this point it's a relief to finally > see the nested ifdefs get removed. > > One naming nit/idea below, but this looks fine as is, so: > > Reviewed-by: John Hubbard Thank you for reviewing this series. > Maybe naming it "movable_page_list", would be a nice

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-03 Thread Jason Gunthorpe
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

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-03 Thread John Hubbard
On 12/1/20 9:23 PM, Pavel Tatashin wrote: We do not allocate pin pages in ZONE_MOVABLE, but if pages were already allocated before pinning they need to migrated to a different zone. Currently, we migrate movable CMA pages only. Generalize the function that migrates CMA pages to migrate all

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-02 Thread Pavel Tatashin
On Wed, Dec 2, 2020 at 8:08 PM Jason Gunthorpe wrote: > > On Wed, Dec 02, 2020 at 07:19:45PM -0500, Pavel Tatashin wrote: > > > It is a good moment to say, I really dislike how this was implemented > > > in the first place. > > > > > > Scanning the output of gup just to do the

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-02 Thread Jason Gunthorpe
On Wed, Dec 02, 2020 at 07:19:45PM -0500, Pavel Tatashin wrote: > > It is a good moment to say, I really dislike how this was implemented > > in the first place. > > > > Scanning the output of gup just to do the is_migrate_movable() test is > > kind of nonsense and slow. It would be better/faster

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-02 Thread Pavel Tatashin
> It is a good moment to say, I really dislike how this was implemented > in the first place. > > Scanning the output of gup just to do the is_migrate_movable() test is > kind of nonsense and slow. It would be better/faster to handle this > directly while gup is scanning the page tables and adding

Re: [PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-02 Thread Jason Gunthorpe
On Wed, Dec 02, 2020 at 12:23:30AM -0500, Pavel Tatashin wrote: > /* > * First define the enums in the above macros to be exported to userspace > diff --git a/mm/gup.c b/mm/gup.c > index 724d8a65e1df..1d511f65f8a7 100644 > +++ b/mm/gup.c > @@ -1593,19 +1593,18 @@ static bool

[PATCH 6/6] mm/gup: migrate pinned pages out of movable zone

2020-12-01 Thread Pavel Tatashin
We do not allocate pin pages in ZONE_MOVABLE, but if pages were already allocated before pinning they need to migrated to a different zone. Currently, we migrate movable CMA pages only. Generalize the function that migrates CMA pages to migrate all movable pages. Signed-off-by: Pavel Tatashin