LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: Mina Almasry <almasrymina@google.com>
Cc: Ken Chen <kenchen@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Chris Kennelly <ckennelly@google.com>
Subject: Re: [PATCH v1] mm, hugepages: add mremap() support for hugepage backed vma
Date: Thu, 5 Aug 2021 09:56:20 -0700	[thread overview]
Message-ID: <86051b4f-491e-1f73-4999-a6c318b4eb2d@oracle.com> (raw)
In-Reply-To: <CAHS8izOCAq-UG1xcSDi2y3N-Cvb0xFnJi5Qcyr_DwPn63VW3VA@mail.gmail.com>

On 8/4/21 11:03 AM, Mina Almasry wrote:
> On Mon, Aug 2, 2021 at 4:51 PM Mike Kravetz <mike.kravetz@oracle.com> wrote:
>> On 7/30/21 3:15 PM, Mina Almasry wrote:
>>> From: Ken Chen <kenchen@google.com>
>>> +static void move_huge_pte(struct vm_area_struct *vma, unsigned long old_addr,
>>> +                       unsigned long new_addr, pte_t *src_pte)
>>> +{
>>> +     struct address_space *mapping = vma->vm_file->f_mapping;
>>> +     struct hstate *h = hstate_vma(vma);
>>> +     struct mm_struct *mm = vma->vm_mm;
>>> +     pte_t *dst_pte, pte;
>>> +     spinlock_t *src_ptl, *dst_ptl;
>>> +
>>> +     /* Shared pagetables need more thought here if we re-enable them */
>>> +     BUG_ON(vma_shareable(vma, old_addr));
>>
>> I agree that shared page tables will complicate the code.  Where do you
>> actually prevent mremap on mappings which can share page tables?  I
>> don't see anything before this BUG.
>>
> 
> Sorry, I added a check in mremap to return early if
> hugetlb_vma_sharable() in v2.
> 

After thinking about this a bit, I am not sure if this is a good idea.
My assumption is that you will make mremap will return an error if
vma_shareable().  We will then need to document that behavior in the
mremap man page.  I 'think' that will require documenting hugetlb pmd
sharing which is not documented anywhere today.

Another option is to 'unshare' early in mremap.  However, unshare will
have the same effect as throwing away all the page table entries for the
shared area.  So, copying page table entries may be very fast.  And, the
first fault on the new vma would theoretically establish sharing again
(assuming all conditions are met).  Otherwise, the new vma will not be
populated until pages are faulted in.  I know mremap wants to preserve
page tables when it remaps.  Does this move us too far from that design
goal?

The last option would be to fully support pmd sharing in the page table
copying code.  It is a bit of a pain, but already accounted for in
routines like copy_hugetlb_page_range.

Just some things to consider.  I would prefer unsharing or fully
supporting sharing rather than return an error.
-- 
Mike Kravetz

  reply	other threads:[~2021-08-05 16:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-30 22:15 Mina Almasry
2021-08-01 19:49 ` Andrew Morton
2021-08-01 23:12   ` Mina Almasry
2021-08-02 23:50 ` Mike Kravetz
2021-08-04 18:03   ` Mina Almasry
2021-08-05 16:56     ` Mike Kravetz [this message]
2021-08-11  0:52     ` Mina Almasry

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=86051b4f-491e-1f73-4999-a6c318b4eb2d@oracle.com \
    --to=mike.kravetz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=almasrymina@google.com \
    --cc=ckennelly@google.com \
    --cc=kenchen@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --subject='Re: [PATCH v1] mm, hugepages: add mremap() support for hugepage backed vma' \
    /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

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).