On 6/4/19 1:11 PM, Dan Williams wrote: > On Tue, Jun 4, 2019 at 12:48 PM John Hubbard <jhubb...@nvidia.com> wrote: >> >> On 6/4/19 9:48 AM, ira.we...@intel.com wrote: >>> From: Ira Weiny <ira.we...@intel.com> >>> ... >>> diff --git a/mm/swap.c b/mm/swap.c >>> index 7ede3eddc12a..6d153ce4cb8c 100644 >>> --- a/mm/swap.c >>> +++ b/mm/swap.c >>> @@ -740,15 +740,20 @@ void release_pages(struct page **pages, int nr) >>> if (is_huge_zero_page(page)) >>> continue; >>> >>> - /* Device public page can not be huge page */ >>> - if (is_device_public_page(page)) { >>> + if (is_zone_device_page(page)) { >>> if (locked_pgdat) { >>> >>> spin_unlock_irqrestore(&locked_pgdat->lru_lock, >>> flags); >>> locked_pgdat = NULL; >>> } >>> - put_devmap_managed_page(page); >>> - continue; >>> + /* >>> + * Not all zone-device-pages require special >>> + * processing. Those pages return 'false' from >>> + * put_devmap_managed_page() expecting a call to >>> + * put_page_testzero() >>> + */ >> >> Just a documentation tweak: how about: >> >> /* >> * ZONE_DEVICE pages that return 'false' from >> * put_devmap_managed_page() do not require special >> * processing, and instead, expect a call to >> * put_page_testzero(). >> */ > > Looks better to me, but maybe just go ahead and list those > expectations explicitly. Something like: > > /* > * put_devmap_managed_page() only handles > * ZONE_DEVICE (struct dev_pagemap managed) > * pages when the hosting dev_pagemap has the > * ->free() or ->fault() callback handlers > * implemented as indicated by > * dev_pagemap.type. Otherwise the expectation > * is to fall back to a plain decrement / > * put_page_testzero(). > */
I like it--but not here, because it's too much internal detail in a call site that doesn't use that level of detail. The call site looks at the return value, only. Let's instead put that blurb above (or in) the put_devmap_managed_page() routine itself. And leave the blurb that I wrote where it is. And then I think everything will have an appropriate level of detail in the right places. thanks, -- John Hubbard NVIDIA