linux-kernel - Re: [PATCH v3 3/5] mm, thp: introduce FOLL_SPLIT

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 linux-cve-announce 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:   Thu, 13 Jun 2019 15:03:01 +0000
From:   Song Liu <songliubraving@...com>
To:     "Kirill A. Shutemov" <kirill@...temov.name>
CC:     LKML <linux-kernel@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "namit@...are.com" <namit@...are.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "oleg@...hat.com" <oleg@...hat.com>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "mhiramat@...nel.org" <mhiramat@...nel.org>,
        "matthew.wilcox@...cle.com" <matthew.wilcox@...cle.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH v3 3/5] mm, thp: introduce FOLL_SPLIT_PMD



> On Jun 13, 2019, at 7:16 AM, Kirill A. Shutemov <kirill@...temov.name> wrote:
> 
> On Thu, Jun 13, 2019 at 01:57:30PM +0000, Song Liu wrote:
>>> And I'm not convinced that it belongs here at all. User requested PMD
>>> split and it is done after split_huge_pmd(). The rest can be handled by
>>> the caller as needed.
>> 
>> I put this part here because split_huge_pmd() for file-backed THP is
>> not really done after split_huge_pmd(). And I would like it done before
>> calling follow_page_pte() below. Maybe we can still do them here, just 
>> for file-backed THPs?
>> 
>> If we would move it, shall we move to callers of follow_page_mask()? 
>> In that case, we will probably end up with similar code in two places:
>> __get_user_pages() and follow_page(). 
>> 
>> Did I get this right?
> 
> Would it be enough to replace pte_offset_map_lock() in follow_page_pte()
> with pte_alloc_map_lock()?

This is similar to my previous version:

+		} else {  /* flags & FOLL_SPLIT_PMD */
+			pte_t *pte;
+			spin_unlock(ptl);
+			split_huge_pmd(vma, pmd, address);
+			pte = get_locked_pte(mm, address, &ptl);
+			if (!pte)
+				return no_page_table(vma, flags);
+			spin_unlock(ptl);
+			ret = 0;
+		}

I think this is cleaner than use pte_alloc_map_lock() in follow_page_pte(). 
What's your thought on these two versions (^^^ vs. pte_alloc_map_lock)?


> This will leave bunch not populated PTE entries, but it is fine: they will
> be populated on the next access to them.

We need to handle page fault during next access, right? Since we already
allocated everything, we can just populate the PTE entries and saves a
lot of page faults (assuming we will access them later). 

Thanks,
Song

> 
> -- 
> Kirill A. Shutemov