All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Hubbard <jhubbard@nvidia.com>
To: Pavel Tatashin <pasha.tatashin@soleen.com>,
	linux-mm <linux-mm@kvack.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	LKML <linux-kernel@vger.kernel.org>,
	Michal Hocko <mhocko@suse.com>,
	"David Hildenbrand" <david@redhat.com>,
	Oscar Salvador <osalvador@suse.de>,
	"Dan Williams" <dan.j.williams@intel.com>,
	Sasha Levin <sashal@kernel.org>,
	"Tyler Hicks" <tyhicks@linux.microsoft.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>, <sthemmin@microsoft.com>
Subject: Re: Pinning ZONE_MOVABLE pages
Date: Mon, 23 Nov 2020 22:49:35 -0800	[thread overview]
Message-ID: <89be454b-4464-0c50-c910-917373f29ba5@nvidia.com> (raw)
In-Reply-To: <CA+CK2bBffHBxjmb9jmSKacm0fJMinyt3Nhk8Nx6iudcQSj80_w@mail.gmail.com>

On 11/20/20 12:27 PM, Pavel Tatashin wrote:
> Recently, I encountered a hang that is happening during memory hot
> remove operation. It turns out that the hang is caused by pinned user
> pages in ZONE_MOVABLE.
> 
> Kernel expects that all pages in ZONE_MOVABLE can be migrated, but
> this is not the case if a user applications such as through dpdk
> libraries pinned them via vfio dma map. Kernel keeps trying to
> hot-remove them, but refcnt never gets to zero, so we are looping
> until the hardware watchdog kicks in.
> 
> We cannot do dma unmaps before hot-remove, because hot-remove is a
> slow operation, and we have thousands for network flows handled by
> dpdk that we just cannot suspend for the duration of hot-remove
> operation.
> 
> The solution is for dpdk to allocate pages from a zone below
> ZONE_MOVAVLE, i.e. ZONE_NORMAL/ZONE_HIGHMEM, but this is not possible.
> There is no user interface that we have that allows applications to
> select what zone the memory should come from.
> 
> I've spoken with Stephen Hemminger, and he said that DPDK is moving in
> the direction of using transparent huge pages instead of HugeTLBs,
> which means that we need to allow at least anonymous, and anonymous
> transparent huge pages to come from non-movable zones on demand.
> 
> Here is what I am proposing:
> 1. Add a new flag that is passed through pin_user_pages_* down to
> fault handlers, and allow the fault handler to allocate from a
> non-movable zone.


I like where the discussion so far (in the other threads) has taken
this. And the current plan also implies, I think, that you can probably
avoid any new flags at all: just check that both FOLL_LONGTERM and
FOLL_PIN are set, and if they are, then make your attempt to migrate
away from ZONE_MOVABLE.



> 
> Sample function stacks through which this info needs to be passed is this:
> 
> pin_user_pages_remote(gup_flags)
>   __get_user_pages_remote(gup_flags)
>    __gup_longterm_locked(gup_flags)
>     __get_user_pages_locked(gup_flags)
>      __get_user_pages(gup_flags)
>       faultin_page(gup_flags)
>        Convert gup_flags into fault_flags
>        handle_mm_fault(fault_flags)

I'm pleased that the gup_flags have pretty much been plumbed through all the
main places that they were missing, so there shouldn't be too much required
at this point.

> 
>  From handle_mm_fault(), the stack diverges into various faults,
> examples include:
> 
> Transparent Huge Page
> handle_mm_fault(fault_flags)
> __handle_mm_fault(fault_flags)
> Create: struct vm_fault vmf, use fault_flags to specify correct gfp_mask
> create_huge_pmd(vmf);
> do_huge_pmd_anonymous_page(vmf);
> mm_get_huge_zero_page(vma->vm_mm); -> flag is lost, so flag from
> vmf.gfp_mask should be passed as well.
> 
> There are several other similar paths in a transparent huge page, also
> there is a named path where allocation is based on filesystems, and
> the flag should be honored there as well, but it does not have to be
> added at the same time.
> 
> Regular Pages
> handle_mm_fault(fault_flags)
> __handle_mm_fault(fault_flags)
> Create: struct vm_fault vmf, use fault_flags to specify correct gfp_mask
> handle_pte_fault(vmf)
> do_anonymous_page(vmf);
> page = alloc_zeroed_user_highpage_movable(vma, vmf->address); ->
> replace change this call according to gfp_mask.
> 
> The above only take care of the case if user application faults on the
> page during pinning time, but there are also cases where pages already
> exist.
> 
> 2. Add an internal move_pages_zone() similar to move_pages() syscall
> but instead of migrating to a different NUMA node, migrate pages from
> ZONE_MOVABLE to another zone.
> Call move_pages_zone() on demand prior to pinning pages from
> vfio_pin_map_dma() for instance.
> 
> 3. Perhaps, it also makes sense to add madvise() flag, to allocate
> pages from non-movable zone. When a user application knows that it
> will do DMA mapping, and pin pages for a long time, the memory that it
> allocates should never be migrated or hot-removed, so make sure that
> it comes from the appropriate place.
> The benefit of adding madvise() flag is that we won't have to deal
> with slow page migration during pin time, but the disadvantage is that
> we would need to change the user interface.
> 
> Before I start working on the above approaches, I would like to get an
> opinion from the community on an appropriate path forward for this
> problem. If what I described sounds reasonable, or if there are other
> ideas on how to address the problem that I am seeing.
> 

I'm also in favor with avoiding (3) for now and maybe forever, depending on
how it goes. Good luck... :)


thanks,
-- 
John Hubbard
NVIDIA

      parent reply	other threads:[~2020-11-24  6:49 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-20 20:27 Pinning ZONE_MOVABLE pages Pavel Tatashin
2020-11-20 20:27 ` Pavel Tatashin
2020-11-20 20:59 ` David Hildenbrand
2020-11-20 21:17   ` Matthew Wilcox
2020-11-20 21:34     ` David Hildenbrand
2020-11-20 21:53       ` Pavel Tatashin
2020-11-20 21:53         ` Pavel Tatashin
2020-11-20 21:58   ` Pavel Tatashin
2020-11-20 21:58     ` Pavel Tatashin
2020-11-20 22:06     ` David Hildenbrand
2020-11-22 21:06 ` David Rientjes
2020-11-22 21:06   ` David Rientjes
2020-11-23 15:31   ` Pavel Tatashin
2020-11-23 15:31     ` Pavel Tatashin
2020-11-23  9:01 ` Michal Hocko
2020-11-23 16:06   ` Pavel Tatashin
2020-11-23 16:06     ` Pavel Tatashin
2020-11-23 17:15     ` Jason Gunthorpe
2020-11-23 17:54       ` Pavel Tatashin
2020-11-23 17:54         ` Pavel Tatashin
2020-11-23 18:34         ` Jason Gunthorpe
2020-11-24  8:20     ` Michal Hocko
2020-11-23 15:04 ` Vlastimil Babka
2020-11-23 16:31   ` Pavel Tatashin
2020-11-23 16:31     ` Pavel Tatashin
2020-11-24  8:24     ` Michal Hocko
2020-11-24  8:43     ` Michal Hocko
2020-11-24  8:44       ` David Hildenbrand
2020-11-24  6:49 ` John Hubbard [this message]

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=89be454b-4464-0c50-c910-917373f29ba5@nvidia.com \
    --to=jhubbard@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=david@redhat.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=osalvador@suse.de \
    --cc=pasha.tatashin@soleen.com \
    --cc=sashal@kernel.org \
    --cc=sthemmin@microsoft.com \
    --cc=tyhicks@linux.microsoft.com \
    --cc=vbabka@suse.cz \
    /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.