All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: Jerome Glisse <jglisse@redhat.com>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>,
	Ingo Molnar <mingo@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linux MM <linux-mm@kvack.org>, Ingo Molnar <mingo@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Logan Gunthorpe <logang@deltatee.com>,
	Kirill Shutemov <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH v2] mm, zone_device: replace {get, put}_zone_device_page() with a single reference
Date: Sun, 30 Apr 2017 19:40:25 -0700	[thread overview]
Message-ID: <CAPcyv4hnDDeAZDEH_8=1xZY99pTW+wh8V714jLEg5J-s2NW6rw@mail.gmail.com> (raw)
In-Reply-To: <20170501015403.GA16181@redhat.com>

On Sun, Apr 30, 2017 at 6:54 PM, Jerome Glisse <jglisse@redhat.com> wrote:
> On Sun, Apr 30, 2017 at 06:42:02PM -0700, Dan Williams wrote:
>> On Sun, Apr 30, 2017 at 4:14 PM, Jerome Glisse <jglisse@redhat.com> wrote:
>> > On Sat, Apr 29, 2017 at 01:17:26PM +0300, Kirill A. Shutemov wrote:
>> >> On Fri, Apr 28, 2017 at 03:33:07PM -0400, Jerome Glisse wrote:
>> >> > On Fri, Apr 28, 2017 at 12:22:24PM -0700, Dan Williams wrote:
>> >> > > Are you sure about needing to hook the 2 -> 1 transition? Could we
>> >> > > change ZONE_DEVICE pages to not have an elevated reference count when
>> >> > > they are created so you can keep the HMM references out of the mm hot
>> >> > > path?
>> >> >
>> >> > 100% sure on that :) I need to callback into driver for 2->1 transition
>> >> > no way around that. If we change ZONE_DEVICE to not have an elevated
>> >> > reference count that you need to make a lot more change to mm so that
>> >> > ZONE_DEVICE is never use as fallback for memory allocation. Also need
>> >> > to make change to be sure that ZONE_DEVICE page never endup in one of
>> >> > the path that try to put them back on lru. There is a lot of place that
>> >> > would need to be updated and it would be highly intrusive and add a
>> >> > lot of special cases to other hot code path.
>> >>
>> >> Could you explain more on where the requirement comes from or point me to
>> >> where I can read about this.
>> >>
>> >
>> > HMM ZONE_DEVICE pages are use like other pages (anonymous or file back page)
>> > in _any_ vma. So i need to know when a page is freed ie either as result of
>> > unmap, exit or migration or anything that would free the memory. For zone
>> > device a page is free once its refcount reach 1 so i need to catch refcount
>> > transition from 2->1
>> >
>> > This is the only way i can inform the device that the page is now free. See
>> >
>> > https://cgit.freedesktop.org/~glisse/linux/commit/?h=hmm-v21&id=52da8fe1a088b87b5321319add79e43b8372ed7d
>> >
>> > There is _no_ way around that.
>>
>> Ok, but I need to point out that this not a ZONE_DEVICE requirement.
>> This is an HMM-specific need. So, this extra reference counting should
>> be clearly delineated as part of the MEMORY_DEVICE_PRIVATE use case.
>
> And it already is delimited, i think you even gave your review by on
> the patch.
>

I gave my reviewed-by to the patch that leveraged
{get,put}_zone_device_page(), but now those are gone and HMM needs a
new patch. I'm just saying these reference counts should not come back
as {get,put}_zone_device_page() because now they're HMM specific.

>> Can we hide the extra reference counting behind a static branch so
>> that the common case fast path doesn't get slower until a HMM device
>> shows up?
>
> Like i already did With likely()/unlikely() ? Or something else ?
>
> https://cgit.freedesktop.org/~glisse/linux/commit/?h=hmm-v21&id=e84778e9db0672e371eb6599dfcb812512118842

Something different:
http://lxr.free-electrons.com/source/Documentation/static-keys.txt

WARNING: multiple messages have this Message-ID (diff)
From: Dan Williams <dan.j.williams@intel.com>
To: Jerome Glisse <jglisse@redhat.com>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>,
	Ingo Molnar <mingo@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linux MM <linux-mm@kvack.org>, Ingo Molnar <mingo@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Logan Gunthorpe <logang@deltatee.com>,
	Kirill Shutemov <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH v2] mm, zone_device: replace {get, put}_zone_device_page() with a single reference
Date: Sun, 30 Apr 2017 19:40:25 -0700	[thread overview]
Message-ID: <CAPcyv4hnDDeAZDEH_8=1xZY99pTW+wh8V714jLEg5J-s2NW6rw@mail.gmail.com> (raw)
In-Reply-To: <20170501015403.GA16181@redhat.com>

On Sun, Apr 30, 2017 at 6:54 PM, Jerome Glisse <jglisse@redhat.com> wrote:
> On Sun, Apr 30, 2017 at 06:42:02PM -0700, Dan Williams wrote:
>> On Sun, Apr 30, 2017 at 4:14 PM, Jerome Glisse <jglisse@redhat.com> wrote:
>> > On Sat, Apr 29, 2017 at 01:17:26PM +0300, Kirill A. Shutemov wrote:
>> >> On Fri, Apr 28, 2017 at 03:33:07PM -0400, Jerome Glisse wrote:
>> >> > On Fri, Apr 28, 2017 at 12:22:24PM -0700, Dan Williams wrote:
>> >> > > Are you sure about needing to hook the 2 -> 1 transition? Could we
>> >> > > change ZONE_DEVICE pages to not have an elevated reference count when
>> >> > > they are created so you can keep the HMM references out of the mm hot
>> >> > > path?
>> >> >
>> >> > 100% sure on that :) I need to callback into driver for 2->1 transition
>> >> > no way around that. If we change ZONE_DEVICE to not have an elevated
>> >> > reference count that you need to make a lot more change to mm so that
>> >> > ZONE_DEVICE is never use as fallback for memory allocation. Also need
>> >> > to make change to be sure that ZONE_DEVICE page never endup in one of
>> >> > the path that try to put them back on lru. There is a lot of place that
>> >> > would need to be updated and it would be highly intrusive and add a
>> >> > lot of special cases to other hot code path.
>> >>
>> >> Could you explain more on where the requirement comes from or point me to
>> >> where I can read about this.
>> >>
>> >
>> > HMM ZONE_DEVICE pages are use like other pages (anonymous or file back page)
>> > in _any_ vma. So i need to know when a page is freed ie either as result of
>> > unmap, exit or migration or anything that would free the memory. For zone
>> > device a page is free once its refcount reach 1 so i need to catch refcount
>> > transition from 2->1
>> >
>> > This is the only way i can inform the device that the page is now free. See
>> >
>> > https://cgit.freedesktop.org/~glisse/linux/commit/?h=hmm-v21&id=52da8fe1a088b87b5321319add79e43b8372ed7d
>> >
>> > There is _no_ way around that.
>>
>> Ok, but I need to point out that this not a ZONE_DEVICE requirement.
>> This is an HMM-specific need. So, this extra reference counting should
>> be clearly delineated as part of the MEMORY_DEVICE_PRIVATE use case.
>
> And it already is delimited, i think you even gave your review by on
> the patch.
>

I gave my reviewed-by to the patch that leveraged
{get,put}_zone_device_page(), but now those are gone and HMM needs a
new patch. I'm just saying these reference counts should not come back
as {get,put}_zone_device_page() because now they're HMM specific.

>> Can we hide the extra reference counting behind a static branch so
>> that the common case fast path doesn't get slower until a HMM device
>> shows up?
>
> Like i already did With likely()/unlikely() ? Or something else ?
>
> https://cgit.freedesktop.org/~glisse/linux/commit/?h=hmm-v21&id=e84778e9db0672e371eb6599dfcb812512118842

Something different:
http://lxr.free-electrons.com/source/Documentation/static-keys.txt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2017-05-01  2:40 UTC|newest]

Thread overview: 91+ 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-23 23:31   ` Kirill A. Shutemov
2017-04-24 17:23   ` Dan Williams
2017-04-24 17:23     ` Dan Williams
2017-04-24 17:30     ` Kirill A. Shutemov
2017-04-24 17:30       ` Kirill A. Shutemov
2017-04-24 17:47       ` Dan Williams
2017-04-24 17:47         ` Dan Williams
2017-04-24 18:01         ` Kirill A. Shutemov
2017-04-24 18:01           ` Kirill A. Shutemov
2017-04-24 18:25           ` Kirill A. Shutemov
2017-04-24 18:25             ` Kirill A. Shutemov
2017-04-24 18:41             ` Dan Williams
2017-04-24 18:41               ` Dan Williams
2017-04-25 13:19               ` Kirill A. Shutemov
2017-04-25 13:19                 ` Kirill A. Shutemov
2017-04-25 16:44                 ` Dan Williams
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  0:55     ` Dan Williams
2017-04-27  8:33     ` Kirill A. Shutemov
2017-04-27  8:33       ` Kirill A. Shutemov
2017-04-28  6:39       ` Ingo Molnar
2017-04-28  6:39         ` Ingo Molnar
2017-04-28  8:14         ` [PATCH] mm, zone_device: Replace " Kirill A. Shutemov
2017-04-28  8:14           ` Kirill A. Shutemov
2017-04-28 17:23         ` [PATCH v2] mm, zone_device: replace " Dan Williams
2017-04-28 17:23           ` Dan Williams
2017-04-28 17:34           ` Jerome Glisse
2017-04-28 17:34             ` Jerome Glisse
2017-04-28 17:41             ` Dan Williams
2017-04-28 17:41               ` Dan Williams
2017-04-28 18:00               ` Jerome Glisse
2017-04-28 18:00                 ` Jerome Glisse
2017-04-28 19:02                 ` Dan Williams
2017-04-28 19:02                   ` Dan Williams
2017-04-28 19:16                   ` Jerome Glisse
2017-04-28 19:16                     ` Jerome Glisse
2017-04-28 19:22                     ` Dan Williams
2017-04-28 19:22                       ` Dan Williams
2017-04-28 19:33                       ` Jerome Glisse
2017-04-28 19:33                         ` Jerome Glisse
2017-04-29 10:17                         ` Kirill A. Shutemov
2017-04-29 10:17                           ` Kirill A. Shutemov
2017-04-30 23:14                           ` Jerome Glisse
2017-04-30 23:14                             ` Jerome Glisse
2017-05-01  1:42                             ` Dan Williams
2017-05-01  1:42                               ` Dan Williams
2017-05-01  1:54                               ` Jerome Glisse
2017-05-01  1:54                                 ` Jerome Glisse
2017-05-01  2:40                                 ` Dan Williams [this message]
2017-05-01  2:40                                   ` Dan Williams
2017-05-01  3:48                             ` Logan Gunthorpe
2017-05-01  3:48                               ` Logan Gunthorpe
2017-05-01 10:23                             ` Kirill A. Shutemov
2017-05-01 10:23                               ` Kirill A. Shutemov
2017-05-01 13:55                               ` Jerome Glisse
2017-05-01 13:55                                 ` Jerome Glisse
2017-05-01 20:19                                 ` Dan Williams
2017-05-01 20:19                                   ` Dan Williams
2017-05-01 20:32                                   ` Jerome Glisse
2017-05-01 20:32                                     ` Jerome Glisse
2017-05-02 11:37                                 ` Kirill A. Shutemov
2017-05-02 11:37                                   ` Kirill A. Shutemov
2017-05-02 13:22                                   ` Jerome Glisse
2017-05-02 13:22                                     ` Jerome Glisse
2017-04-29 14:18           ` Ingo Molnar
2017-04-29 14:18             ` Ingo Molnar
2017-05-01  2:45             ` Dan Williams
2017-05-01  2:45               ` Dan Williams
2017-05-01  7:12               ` Ingo Molnar
2017-05-01  7:12                 ` Ingo Molnar
2017-05-01  9:33                 ` Kirill A. Shutemov
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:11       ` Logan Gunthorpe
2017-04-27 16:14       ` Dan Williams
2017-04-27 16:14         ` Dan Williams
2017-04-27 16:33         ` Logan Gunthorpe
2017-04-27 16:33           ` Logan Gunthorpe
2017-04-27 16:38           ` Dan Williams
2017-04-27 16:38             ` Dan Williams
2017-04-27 16:45             ` Logan Gunthorpe
2017-04-27 16:45               ` Logan Gunthorpe
2017-04-27 16:46               ` Dan Williams
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='CAPcyv4hnDDeAZDEH_8=1xZY99pTW+wh8V714jLEg5J-s2NW6rw@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=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=logang@deltatee.com \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.