search for: __put_page

...> and use some other sign in the struct page that offlining is permitted. Few questions. I do not see onlining code to take care of this special case. What should happen when offline && online? Should we allow to try_remove_memory to succeed with these pages? Do we really have hook into __put_page? Why do we even care about the reference count of those pages? Wouldn't it be just more consistent to elevate the reference count (I guess this is what you suggest in the last paragraph) and the virtio driver would return that page to the buddy by regular put_page. This is also related to the a...

...> and use some other sign in the struct page that offlining is permitted. Few questions. I do not see onlining code to take care of this special case. What should happen when offline && online? Should we allow to try_remove_memory to succeed with these pages? Do we really have hook into __put_page? Why do we even care about the reference count of those pages? Wouldn't it be just more consistent to elevate the reference count (I guess this is what you suggest in the last paragraph) and the virtio driver would return that page to the buddy by regular put_page. This is also related to the a...

...ing to really nail down how to deal with reference counting here because it is an important aspect. > >>> explicit control via the reference count which is the standard way to > >>> control the struct page life cycle. > >>> > >>> Anyway hooking into __put_page (which tends to be a hot path with > >>> something that is barely used on most systems) doesn't sound nice to me. > >>> This is the whole point which made me think about the whole reference > >>> count approach in the first place. > >> > >>...

...ing to really nail down how to deal with reference counting here because it is an important aspect. > >>> explicit control via the reference count which is the standard way to > >>> control the struct page life cycle. > >>> > >>> Anyway hooking into __put_page (which tends to be a hot path with > >>> something that is barely used on most systems) doesn't sound nice to me. > >>> This is the whole point which made me think about the whole reference > >>> count approach in the first place. > >> > >>...

...at offlining is permitted. > > > > Few questions. I do not see onlining code to take care of this special > > case. What should happen when offline && online? > > Should we allow to try_remove_memory to succeed with these pages? > > Do we really have hook into __put_page? Why do we even care about the > > reference count of those pages? > > Oh, I forgot to answer this questions. The __put_page() change is necessary > for the following race I identified: > > Page has a refcount of 1 (e.g., allocated by virtio-mem using > alloc_contig_range(...

...at offlining is permitted. > > > > Few questions. I do not see onlining code to take care of this special > > case. What should happen when offline && online? > > Should we allow to try_remove_memory to succeed with these pages? > > Do we really have hook into __put_page? Why do we even care about the > > reference count of those pages? > > Oh, I forgot to answer this questions. The __put_page() change is necessary > for the following race I identified: > > Page has a refcount of 1 (e.g., allocated by virtio-mem using > alloc_contig_range(...

...you do not want to release that page from the notifier context to reflect that the page is ok to be offlined with the rest. [...] > > explicit control via the reference count which is the standard way to > > control the struct page life cycle. > > > > Anyway hooking into __put_page (which tends to be a hot path with > > something that is barely used on most systems) doesn't sound nice to me. > > This is the whole point which made me think about the whole reference > > count approach in the first place. > > Again, the race I think that is possible...

...you do not want to release that page from the notifier context to reflect that the page is ok to be offlined with the rest. [...] > > explicit control via the reference count which is the standard way to > > control the struct page life cycle. > > > > Anyway hooking into __put_page (which tends to be a hot path with > > something that is barely used on most systems) doesn't sound nice to me. > > This is the whole point which made me think about the whole reference > > count approach in the first place. > > Again, the race I think that is possible...

...ign in the struct page that offlining is permitted. > > Few questions. I do not see onlining code to take care of this special > case. What should happen when offline && online? > Should we allow to try_remove_memory to succeed with these pages? > Do we really have hook into __put_page? Why do we even care about the > reference count of those pages? Oh, I forgot to answer this questions. The __put_page() change is necessary for the following race I identified: Page has a refcount of 1 (e.g., allocated by virtio-mem using alloc_contig_range()). a) kernel: get_page_unless_z...

...1, when refcount reach one it means the page is free and we - * need to inform the device driver through callback. See - * include/linux/memremap.h and HMM for details. - */ - if (page_is_devmap_managed(page)) { - put_devmap_managed_page(page); - return; - } - if (put_page_testzero(page)) __put_page(page); } diff --git a/lib/test_hmm.c b/lib/test_hmm.c index e7dc3de355b7..1033b19c9c52 100644 --- a/lib/test_hmm.c +++ b/lib/test_hmm.c @@ -561,7 +561,7 @@ static struct page *dmirror_devmem_alloc_page(struct dmirror_device *mdevice) } dpage->zone_device_data = rpage; - get_page(dpage); +...

...1, when refcount reach one it means the page is free and we - * need to inform the device driver through callback. See - * include/linux/memremap.h and HMM for details. - */ - if (page_is_devmap_managed(page)) { - put_devmap_managed_page(page); - return; - } - if (put_page_testzero(page)) __put_page(page); } diff --git a/lib/test_hmm.c b/lib/test_hmm.c index e151a7f10519..37a2dea5e805 100644 --- a/lib/test_hmm.c +++ b/lib/test_hmm.c @@ -561,7 +561,7 @@ static struct page *dmirror_devmem_alloc_page(struct dmirror_device *mdevice) } dpage->zone_device_data = rpage; - get_page(dpage); +...

...is_device_private_page(page); if (mapping) expected_count += thp_nr_pages(page) + page_has_private(page); diff --git a/mm/swap.c b/mm/swap.c index 0eb057141a04..93d880c6f73c 100644 --- a/mm/swap.c +++ b/mm/swap.c @@ -116,12 +116,11 @@ static void __put_compound_page(struct page *page) void __put_page(struct page *page) { if (is_zone_device_page(page)) { - put_dev_pagemap(page->pgmap); - /* * The page belongs to the device that created pgmap. Do * not return it to page allocator. */ + free_zone_device_page(page); return; } @@ -907,6 +906,9 @@ void release_pages(stru...

...>> notifiers) while incrementing the reference count. Also, a driver that >> relies in this feature is aware that re-onlining the memory will require >> to re-set the pages PageOffline() - e.g., via the online_page_callback_t." > > OK > > [...] >> d) __put_page() is modified to not return pages to the buddy in any >> case as a safety net. We might be able to get rid of that. > > I do not like exactly this part Yeah, and I think I can drop it from this patch. > >> What I think you suggest: >> >> a) has_unmovable_p...

...s permitted. >>> >>> Few questions. I do not see onlining code to take care of this special >>> case. What should happen when offline && online? >>> Should we allow to try_remove_memory to succeed with these pages? >>> Do we really have hook into __put_page? Why do we even care about the >>> reference count of those pages? >> >> Oh, I forgot to answer this questions. The __put_page() change is necessary >> for the following race I identified: >> >> Page has a refcount of 1 (e.g., allocated by virtio-mem using &gt...

...ok, but just to quick question. try_remove_memory would fail if the range is offline (via user interface) but there are still some pages in the driver Offline state? > Once offline, the memmap is irrelevant and try_remove_memory() can do its > job. > > > Do we really have hook into __put_page? Why do we even care about the > > reference count of those pages? Wouldn't it be just more consistent to > > elevate the reference count (I guess this is what you suggest in the > > last paragraph) and the virtio driver would return that page to the > > buddy by regular...

...ok, but just to quick question. try_remove_memory would fail if the range is offline (via user interface) but there are still some pages in the driver Offline state? > Once offline, the memmap is irrelevant and try_remove_memory() can do its > job. > > > Do we really have hook into __put_page? Why do we even care about the > > reference count of those pages? Wouldn't it be just more consistent to > > elevate the reference count (I guess this is what you suggest in the > > last paragraph) and the virtio driver would return that page to the > > buddy by regular...

...1, when refcount reach one it means the page is free and we - * need to inform the device driver through callback. See - * include/linux/memremap.h and HMM for details. - */ - if (page_is_devmap_managed(page)) { - put_devmap_managed_page(page); - return; - } - if (put_page_testzero(page)) __put_page(page); } diff --git a/lib/test_hmm.c b/lib/test_hmm.c index c8133f50160b..5410f7328577 100644 --- a/lib/test_hmm.c +++ b/lib/test_hmm.c @@ -561,7 +561,6 @@ static struct page *dmirror_devmem_alloc_page(struct dmirror_device *mdevice) } dpage->zone_device_data = rpage; - get_page(dpage);...

...is garbage), it only makes sure that the memory blocks are properly marked as offline. (IOW, device_offline() was called on the memory block). > >> Once offline, the memmap is irrelevant and try_remove_memory() can do its >> job. >> >>> Do we really have hook into __put_page? Why do we even care about the >>> reference count of those pages? Wouldn't it be just more consistent to >>> elevate the reference count (I guess this is what you suggest in the >>> last paragraph) and the virtio driver would return that page to the >>> budd...

...tand this right now. But maybe I will get it after >> reading the patches and suggest some improved wording here. > > I will try to explain why such rule happens in there. > > The problem is that put_page is aware of PageLRU. So, if someone releases > last refcount of LRU page, __put_page checks PageLRU and then, clear the > flags and detatch the page in LRU list(i.e., data structure). > But in case of driver page, data structure like LRU among drivers is not only one. > IOW, we should add following code in put_page to handle various requirements > of driver page. > &...

Matthew Wilcox, Ira Weiny, and others have complained that ZONE_DEVICE struct page reference counting is ugly because they are "free" when the reference count is one instead of zero. This leads to explicit checks for ZONE_DEVICE pages in places like put_page(), GUP, THP splitting, and page migration which have to adjust the expected reference count when determining if the page is