All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alistair Popple <apopple@nvidia.com>
To: John Hubbard <jhubbard@nvidia.com>
Cc: alex.sierra@amd.com, rcampbell@nvidia.com, willy@infradead.org,
	Felix.Kuehling@amd.com, amd-gfx@lists.freedesktop.org,
	linux-xfs@vger.kernel.org, linux-mm@kvack.org,
	jglisse@redhat.com, dri-devel@lists.freedesktop.org,
	jgg@nvidia.com, akpm@linux-foundation.org,
	linux-ext4@vger.kernel.org, hch@lst.de
Subject: Re: [PATCH v2 2/3] mm/gup.c: Migrate device coherent pages when pinning instead of failing
Date: Mon, 14 Feb 2022 11:05:50 +1100	[thread overview]
Message-ID: <877d9ygvdj.fsf@nvdebian.thelocal> (raw)
In-Reply-To: <0c7b50d3-9bca-9462-05ca-b4ef846cc49a@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 1447 bytes --]

John Hubbard <jhubbard@nvidia.com> writes:

> On 2/11/22 18:51, Alistair Popple wrote:

[…]

>>> See below…
>>>
>>>> +			}
>>>> +
>>>> +			pages[i] = migrate_device_page(head, gup_flags);
>> migrate_device_page() will return a new page that has been correctly pinned
>> with gup_flags by try_grab_page(). Therefore this page can still be released
>> with unpin_user_page() or put_page() as appropriate for the given gup_flags.
>> The reference we had on the source page (head) always gets dropped in
>> migrate_vma_finalize().
>
> OK. Good.
>
> The above would be good to have in a comment, right around here, imho.
> Because we have this marvelous mix of references for migration (get_page())
> and other, and it’s a bit hard to see that it’s all correct without a
> hint or two.

Ok, will do.

>
> …
>> Which unless I’ve missed something is still the correct thing to do.
>>
>>> This reminds me: out of the many things to monitor, the FOLL_PIN counts
>>> in /proc/vmstat are especially helpful, whenever making changes to code
>>> that deals with this:
>>>
>>> 	nr_foll_pin_acquired
>>> 	nr_foll_pin_released
>>>
>>> …and those should normally be equal to each other when “at rest”.
>>>
>
> I hope this is/was run, just to be sure?

Thanks for the suggestion, these remain equal to each other after running
hmm-tests which confirms everything is working as expected.

>
> thanks,

WARNING: multiple messages have this Message-ID (diff)
From: Alistair Popple <apopple@nvidia.com>
To: John Hubbard <jhubbard@nvidia.com>
Cc: akpm@linux-foundation.org, linux-mm@kvack.org,
	Felix.Kuehling@amd.com, rcampbell@nvidia.com,
	linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	hch@lst.de, jgg@nvidia.com, jglisse@redhat.com,
	willy@infradead.org, alex.sierra@amd.com
Subject: Re: [PATCH v2 2/3] mm/gup.c: Migrate device coherent pages when pinning instead of failing
Date: Mon, 14 Feb 2022 11:05:50 +1100	[thread overview]
Message-ID: <877d9ygvdj.fsf@nvdebian.thelocal> (raw)
In-Reply-To: <0c7b50d3-9bca-9462-05ca-b4ef846cc49a@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 1447 bytes --]

John Hubbard <jhubbard@nvidia.com> writes:

> On 2/11/22 18:51, Alistair Popple wrote:

[…]

>>> See below…
>>>
>>>> +			}
>>>> +
>>>> +			pages[i] = migrate_device_page(head, gup_flags);
>> migrate_device_page() will return a new page that has been correctly pinned
>> with gup_flags by try_grab_page(). Therefore this page can still be released
>> with unpin_user_page() or put_page() as appropriate for the given gup_flags.
>> The reference we had on the source page (head) always gets dropped in
>> migrate_vma_finalize().
>
> OK. Good.
>
> The above would be good to have in a comment, right around here, imho.
> Because we have this marvelous mix of references for migration (get_page())
> and other, and it’s a bit hard to see that it’s all correct without a
> hint or two.

Ok, will do.

>
> …
>> Which unless I’ve missed something is still the correct thing to do.
>>
>>> This reminds me: out of the many things to monitor, the FOLL_PIN counts
>>> in /proc/vmstat are especially helpful, whenever making changes to code
>>> that deals with this:
>>>
>>> 	nr_foll_pin_acquired
>>> 	nr_foll_pin_released
>>>
>>> …and those should normally be equal to each other when “at rest”.
>>>
>
> I hope this is/was run, just to be sure?

Thanks for the suggestion, these remain equal to each other after running
hmm-tests which confirms everything is working as expected.

>
> thanks,

  reply	other threads:[~2022-02-14  8:44 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-07  4:26 [PATCH v2 0/3] Migrate device coherent pages on get_user_pages() Alistair Popple
2022-02-07  4:26 ` Alistair Popple
2022-02-07  4:26 ` [PATCH v2 1/3] migrate.c: Remove vma check in migrate_vma_setup() Alistair Popple
2022-02-07  4:26   ` Alistair Popple
2022-02-15 21:50   ` John Hubbard
2022-02-15 21:50     ` John Hubbard
2022-02-07  4:26 ` [PATCH v2 2/3] mm/gup.c: Migrate device coherent pages when pinning instead of failing Alistair Popple
2022-02-07  4:26   ` Alistair Popple
2022-02-10 10:53   ` David Hildenbrand
2022-02-10 10:53     ` David Hildenbrand
2022-02-10 11:39     ` Alistair Popple
2022-02-10 11:39       ` Alistair Popple
2022-02-10 11:47       ` David Hildenbrand
2022-02-10 11:47         ` David Hildenbrand
2022-02-10 23:41         ` Alistair Popple
2022-02-10 23:41           ` Alistair Popple
2022-02-15 12:03           ` David Hildenbrand
2022-02-15 12:03             ` David Hildenbrand
2022-02-12  2:10   ` John Hubbard
2022-02-12  2:10     ` John Hubbard
2022-02-12  2:51     ` Alistair Popple
2022-02-12  2:51       ` Alistair Popple
2022-02-12  3:04       ` John Hubbard
2022-02-12  3:04         ` John Hubbard
2022-02-14  0:05         ` Alistair Popple [this message]
2022-02-14  0:05           ` Alistair Popple
2022-02-07  4:26 ` [PATCH v2 3/3] tools: add hmm gup test for long term pinned device pages Alistair Popple
2022-02-07  4:26   ` Alistair Popple

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=877d9ygvdj.fsf@nvdebian.thelocal \
    --to=apopple@nvidia.com \
    --cc=Felix.Kuehling@amd.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex.sierra@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hch@lst.de \
    --cc=jgg@nvidia.com \
    --cc=jglisse@redhat.com \
    --cc=jhubbard@nvidia.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=rcampbell@nvidia.com \
    --cc=willy@infradead.org \
    /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.