From: Dan Williams <dan.j.williams@intel.com>
To: Logan Gunthorpe <logang@deltatee.com>
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
"Linux MM" <linux-mm@kvack.org>,
"Jérôme Glisse" <jglisse@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Kirill Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH] mm, zone_device: replace {get, put}_zone_device_page() with a single reference
Date: Thu, 27 Apr 2017 09:14:09 -0700 [thread overview]
Message-ID: <CAPcyv4it4eGhLjws_j8+M1BeAzr_gHRZ4zE-nC+4QMpFp72Hyg@mail.gmail.com> (raw)
In-Reply-To: <3e595ba6-2ea1-e25d-e254-6c7edcf23f88@deltatee.com>
On Thu, Apr 27, 2017 at 9:11 AM, Logan Gunthorpe <logang@deltatee.com> wrote:
>
>
> On 26/04/17 06:55 PM, Dan Williams wrote:
>> @@ -277,7 +269,10 @@ struct dev_pagemap *find_dev_pagemap(resource_size_t phys)
>> *
>> * Notes:
>> * 1/ @ref must be 'live' on entry and 'dead' before devm_memunmap_pages() time
>> - * (or devm release event).
>> + * (or devm release event). The expected order of events is that @ref has
>> + * been through percpu_ref_kill() before devm_memremap_pages_release(). The
>> + * wait for the completion of kill and percpu_ref_exit() must occur after
>> + * devm_memremap_pages_release().
>> *
>> * 2/ @res is expected to be a host memory range that could feasibly be
>> * treated as a "System RAM" range, i.e. not a device mmio range, but
>> @@ -379,6 +374,7 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
>> */
>> list_del(&page->lru);
>> page->pgmap = pgmap;
>> + percpu_ref_get(ref);
>> }
>> devres_add(dev, page_map);
>> return __va(res->start);
>> diff --git a/mm/swap.c b/mm/swap.c
>> index 5dabf444d724..01267dda6668 100644
>> --- a/mm/swap.c
>> +++ b/mm/swap.c
>> @@ -97,6 +97,16 @@ 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 belong to device, do not return it to
>> + * page allocator.
>> + */
>> + return;
>> + }
>> +
>> if (unlikely(PageCompound(page)))
>> __put_compound_page(page);
>> else
>>
>
> Forgive me if I'm missing something but this doesn't make sense to me.
> We are taking a reference once when the region is initialized and
> releasing it every time a page within the region's reference count drops
> to zero. That does not seem to be symmetric and I don't see how it
> tracks that pages are in use. Shouldn't get_dev_pagemap be called when
> any page is allocated or something like that (ie. the inverse of
> __put_page)?
You're overlooking that the page reference count 1 after
arch_add_memory(). So at the end of time we're just dropping the
arch_add_memory() reference to release the page and related
dev_pagemap.
next prev parent reply other threads:[~2017-04-27 16:14 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-20 21:46 [tip:x86/mm] x86/mm/gup: Switch GUP to the generic get_user_page_fast() implementation Dan Williams
2017-04-21 14:16 ` Kirill A. Shutemov
2017-04-21 19:30 ` Dan Williams
2017-04-23 9:52 ` [PATCH] Revert "x86/mm/gup: Switch GUP to the generic get_user_page_fast() implementation" Ingo Molnar
2017-04-23 23:31 ` get_zone_device_page() in get_page() and page_cache_get_speculative() Kirill A. Shutemov
2017-04-24 17:23 ` Dan Williams
2017-04-24 17:30 ` Kirill A. Shutemov
2017-04-24 17:47 ` Dan Williams
2017-04-24 18:01 ` Kirill A. Shutemov
2017-04-24 18:25 ` Kirill A. Shutemov
2017-04-24 18:41 ` Dan Williams
2017-04-25 13:19 ` Kirill A. Shutemov
2017-04-25 16:44 ` Dan Williams
2017-04-27 0:55 ` [PATCH] mm, zone_device: replace {get, put}_zone_device_page() with a single reference Dan Williams
2017-04-27 8:33 ` Kirill A. Shutemov
2017-04-28 6:39 ` Ingo Molnar
2017-04-28 8:14 ` [PATCH] mm, zone_device: Replace " Kirill A. Shutemov
2017-04-28 17:23 ` [PATCH v2] mm, zone_device: replace " Dan Williams
2017-04-28 17:34 ` Jerome Glisse
2017-04-28 17:41 ` Dan Williams
2017-04-28 18:00 ` Jerome Glisse
2017-04-28 19:02 ` Dan Williams
2017-04-28 19:16 ` Jerome Glisse
2017-04-28 19:22 ` Dan Williams
2017-04-28 19:33 ` Jerome Glisse
2017-04-29 10:17 ` Kirill A. Shutemov
2017-04-30 23:14 ` Jerome Glisse
2017-05-01 1:42 ` Dan Williams
2017-05-01 1:54 ` Jerome Glisse
2017-05-01 2:40 ` Dan Williams
2017-05-01 3:48 ` Logan Gunthorpe
2017-05-01 10:23 ` Kirill A. Shutemov
2017-05-01 13:55 ` Jerome Glisse
2017-05-01 20:19 ` Dan Williams
2017-05-01 20:32 ` Jerome Glisse
2017-05-02 11:37 ` Kirill A. Shutemov
2017-05-02 13:22 ` Jerome Glisse
2017-04-29 14:18 ` Ingo Molnar
2017-05-01 2:45 ` Dan Williams
2017-05-01 7:12 ` Ingo Molnar
2017-05-01 9:33 ` Kirill A. Shutemov
2017-05-01 8:28 ` [tip:x86/mm] mm, zone_device: Replace {get, put}_zone_device_page() with a single reference to fix pmem crash tip-bot for Dan Williams
2017-04-27 16:11 ` [PATCH] mm, zone_device: replace {get, put}_zone_device_page() with a single reference Logan Gunthorpe
2017-04-27 16:14 ` Dan Williams [this message]
2017-04-27 16:33 ` Logan Gunthorpe
2017-04-27 16:38 ` Dan Williams
2017-04-27 16:45 ` Logan Gunthorpe
2017-04-27 16:46 ` Dan Williams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPcyv4it4eGhLjws_j8+M1BeAzr_gHRZ4zE-nC+4QMpFp72Hyg@mail.gmail.com \
--to=dan.j.williams@intel.com \
--cc=akpm@linux-foundation.org \
--cc=jglisse@redhat.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=logang@deltatee.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).