lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 13 Jan 2021 15:27:31 -0800
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Oscar Salvador <osalvador@...e.de>,
        Muchun Song <songmuchun@...edance.com>
Cc:     Jonathan Corbet <corbet@....net>,
        Thomas Gleixner <tglx@...utronix.de>, mingo@...hat.com,
        bp@...en8.de, x86@...nel.org, hpa@...or.com,
        dave.hansen@...ux.intel.com, luto@...nel.org,
        Peter Zijlstra <peterz@...radead.org>, viro@...iv.linux.org.uk,
        Andrew Morton <akpm@...ux-foundation.org>, paulmck@...nel.org,
        mchehab+huawei@...nel.org, pawan.kumar.gupta@...ux.intel.com,
        Randy Dunlap <rdunlap@...radead.org>, oneukum@...e.com,
        anshuman.khandual@....com, jroedel@...e.de,
        Mina Almasry <almasrymina@...gle.com>,
        David Rientjes <rientjes@...gle.com>,
        Matthew Wilcox <willy@...radead.org>,
        Michal Hocko <mhocko@...e.com>,
        "Song Bao Hua (Barry Song)" <song.bao.hua@...ilicon.com>,
        David Hildenbrand <david@...hat.com>,
        HORIGUCHI NAOYA(堀口 直也) 
        <naoya.horiguchi@....com>,
        Xiongchun duan <duanxiongchun@...edance.com>,
        linux-doc@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
        Linux Memory Management List <linux-mm@...ck.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [External] Re: [PATCH v12 04/13] mm/hugetlb: Free the vmemmap
 pages associated with each HugeTLB page

On 1/13/21 1:20 AM, Oscar Salvador wrote:
> On Tue, Jan 12, 2021 at 07:33:33PM +0800, Muchun Song wrote:
>>> It seems a bit odd to only pass "start" for the BUG_ON.
>>> Also, I kind of dislike the "addr += PAGE_SIZE" in vmemmap_pte_range.
>>>
>>> I wonder if adding a ".remap_start_addr" would make more sense.
>>> And adding it here with the vmemmap_remap_walk init.
>>
>> How about introducing a new function which aims to get the reuse
>> page? In this case, we can drop the BUG_ON() and "addr += PAGE_SIZE"
>> which is in vmemmap_pte_range. The vmemmap_remap_range only
>> does the remapping.
> 
> How would that look? 
> It might be good, dunno, but the point is, we should try to make the rules as
> simple as possible, dropping weird assumptions.
> 
> Callers of vmemmap_remap_free should know three things:
> 
> - Range to be remapped
> - Addr to remap to
> - Current implemantion needs addr to be remap to to be part of the complete
>   range
> 
> right?

And, current implementation needs must have remap addr be the first in the
complete range.  This is just because of the way the page tables are walked
for remapping.  The remap/reuse page must be found first so that the following
pages can be remapped to it.

That implementation seems to be the 'most efficient' for hugetlb pages where
we want vmemmap pages n+3 and beyond mapped to n+2.

In a more general purpose vmemmap_remap_free implementation, the reuse/remap
address would not necessarily need to be related to the range.  However, this
would require a separate page table walk/validation for the reuse address
independent of the range.  This may be what Muchun was proposing for 'a new
function which aims to get the reuse page'.

IMO, the decision on how to implement depends on the intended use case.
- If this is going to be hugetlb only (or perhaps generic huge page only)
  functionality, then I am OK with an efficient implementation that has
  some restrictions.
- If we see this being used for more general purpose remapping, then we
  should go with a more general purpose implementation.

Again, just my opinion.
-- 
Mike Kravetz

Powered by blists - more mailing lists