All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alistair Popple <apopple@nvidia.com>
To: John Hubbard <jhubbard@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-mm@kvack.org, David Hildenbrand <david@redhat.com>,
	Shigeru Yoshida <syoshida@redhat.com>,
	Jason Gunthorpe <jgg@nvidia.com>,
	Minchan Kim <minchan@kernel.org>,
	Pasha Tatashin <pasha.tatashin@soleen.com>
Subject: Re: [PATCH v3] mm/gup: stop leaking pinned pages in low memory conditions
Date: Mon, 21 Oct 2024 16:39:55 +1100	[thread overview]
Message-ID: <87ttd6atxi.fsf@nvdebian.thelocal> (raw)
In-Reply-To: <142152a5-d265-4aa5-b103-dede882f9715@nvidia.com>


John Hubbard <jhubbard@nvidia.com> writes:

> On 10/20/24 4:26 PM, Alistair Popple wrote:
>> John Hubbard <jhubbard@nvidia.com> writes:
>> [...]
>>> @@ -2437,8 +2440,10 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages,
>>>   	long i, ret;
>>>     	folios = kmalloc_array(nr_pages, sizeof(*folios), GFP_KERNEL);
>>> -	if (!folios)
>>> +	if (!folios) {
>>> +		unpin_user_pages(pages, nr_pages);
>> ie. Doesn't this unpinning need to happen in
>> check_and_migrate_movable_folios()?
>
> It already does.
>
> check_and_migrate_movable_folios() calls
> migrate_longterm_unpinnable_folios(), which unpins if errors occur.

Right you are.

Reviewed-by: Alistair Popple <apopple@nvidia.com>

As an aside for future clean-ups we could probably get something nicer
if we reversed the process of pin/migrate to migrate/pin. In other words
if FOLL_LONGERM try and migrate the entire range first out of
ZONE_MOVABLE first. Migration invovles walking page tables and getting a
reference on the pages anyway, so if it turns out there is nothing to
migrate you haven't lost anything performance wise.

> thanks,



  reply	other threads:[~2024-10-21  5:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-18 22:34 [PATCH v3] mm/gup: stop leaking pinned pages in low memory conditions John Hubbard
2024-10-20 23:26 ` Alistair Popple
2024-10-21  3:26   ` John Hubbard
2024-10-21  5:39     ` Alistair Popple [this message]
2024-10-21  6:38       ` John Hubbard

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=87ttd6atxi.fsf@nvdebian.thelocal \
    --to=apopple@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=jgg@nvidia.com \
    --cc=jhubbard@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=pasha.tatashin@soleen.com \
    --cc=syoshida@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.