| Message ID | 20191121071354.456618-6-jhubbard@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/gup: track dma-pinned pages: FOLL_PIN | expand |
On 11/21/19 12:05 AM, Christoph Hellwig wrote: > So while this looks correct and I still really don't see the major > benefit of the new code organization, especially as it bloats all > put_page callers. > > I'd love to see code size change stats for an allyesconfig on this > commit. > Right, I'm running that now, will post the results. (btw, if there is a script and/or standard format I should use, I'm all ears. I'll dig through lwn...) thanks,
On Thu, Nov 21, 2019 at 12:57 AM John Hubbard <jhubbard@nvidia.com> wrote: > > On 11/21/19 12:05 AM, Christoph Hellwig wrote: > > So while this looks correct and I still really don't see the major > > benefit of the new code organization, especially as it bloats all > > put_page callers. > > > > I'd love to see code size change stats for an allyesconfig on this > > commit. > > > > Right, I'm running that now, will post the results. (btw, if there is > a script and/or standard format I should use, I'm all ears. I'll dig > through lwn...) > Just run: size vmlinux
On 11/21/19 8:59 AM, Dan Williams wrote: > On Thu, Nov 21, 2019 at 12:57 AM John Hubbard <jhubbard@nvidia.com> wrote: >> >> On 11/21/19 12:05 AM, Christoph Hellwig wrote: >>> So while this looks correct and I still really don't see the major >>> benefit of the new code organization, especially as it bloats all >>> put_page callers. >>> >>> I'd love to see code size change stats for an allyesconfig on this >>> commit. >>> >> >> Right, I'm running that now, will post the results. (btw, if there is >> a script and/or standard format I should use, I'm all ears. I'll dig >> through lwn...) >> > > Just run: > > size vmlinux > Beautiful. I thought it would involve a lot more. Here's results: linux.git (Linux 5.4-rc8+): ============================================== text data bss dec hex filename 227578032 213267935 76877984 517723951 1edbd72f vmlinux With patches 4 and 5 applied to linux.git: ========================================== text data bss dec hex filename 229698560 213288379 76853408 519840347 1efc225b vmlinux Analysis: ========= This increased the size of text by 0.93%. Which is a measurable bloat, so the inlining really is undesirable here, yes. I'll do it differently. thanks,
diff --git a/include/linux/mm.h b/include/linux/mm.h index a2adf95b3f9c..96228376139c 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -967,9 +967,10 @@ static inline bool is_zone_device_page(const struct page *page) #endif #ifdef CONFIG_DEV_PAGEMAP_OPS -void __put_devmap_managed_page(struct page *page); +void free_devmap_managed_page(struct page *page); DECLARE_STATIC_KEY_FALSE(devmap_managed_key); -static inline bool put_devmap_managed_page(struct page *page) + +static inline bool page_is_devmap_managed(struct page *page) { if (!static_branch_unlikely(&devmap_managed_key)) return false; @@ -978,7 +979,6 @@ static inline bool put_devmap_managed_page(struct page *page) switch (page->pgmap->type) { case MEMORY_DEVICE_PRIVATE: case MEMORY_DEVICE_FS_DAX: - __put_devmap_managed_page(page); return true; default: break; @@ -986,6 +986,27 @@ static inline bool put_devmap_managed_page(struct page *page) return false; } +static inline bool put_devmap_managed_page(struct page *page) +{ + bool is_devmap = page_is_devmap_managed(page); + + if (is_devmap) { + int count = page_ref_dec_return(page); + + /* + * devmap page refcounts are 1-based, rather than 0-based: if + * refcount is 1, then the page is free and the refcount is + * stable because nobody holds a reference on the page. + */ + if (count == 1) + free_devmap_managed_page(page); + else if (!count) + __put_page(page); + } + + return is_devmap; +} + #else /* CONFIG_DEV_PAGEMAP_OPS */ static inline bool put_devmap_managed_page(struct page *page) { diff --git a/mm/memremap.c b/mm/memremap.c index e899fa876a62..2ba773859031 100644 --- a/mm/memremap.c +++ b/mm/memremap.c @@ -411,20 +411,8 @@ struct dev_pagemap *get_dev_pagemap(unsigned long pfn, EXPORT_SYMBOL_GPL(get_dev_pagemap); #ifdef CONFIG_DEV_PAGEMAP_OPS -void __put_devmap_managed_page(struct page *page) +void free_devmap_managed_page(struct page *page) { - int count = page_ref_dec_return(page); - - /* still busy */ - if (count > 1) - return; - - /* only triggered by the dev_pagemap shutdown path */ - if (count == 0) { - __put_page(page); - return; - } - /* notify page idle for dax */ if (!is_device_private_page(page)) { wake_up_var(&page->_refcount); @@ -461,5 +449,5 @@ void __put_devmap_managed_page(struct page *page) page->mapping = NULL; page->pgmap->ops->page_free(page); } -EXPORT_SYMBOL(__put_devmap_managed_page); +EXPORT_SYMBOL(free_devmap_managed_page); #endif /* CONFIG_DEV_PAGEMAP_OPS */