linux-kernel - Re: [PATCH v2 5/9] mm/mshare: Add vm flag for shared PTE

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]
Message-ID: <63d8437e-5e33-5d83-de86-779f5d334ba3@oracle.com>
Date:   Thu, 30 Jun 2022 09:46:57 -0600
From:   Khalid Aziz <khalid.aziz@...cle.com>
To:     Mark Hemment <markhemm@...glemail.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        aneesh.kumar@...ux.ibm.com, arnd@...db.de, 21cnbao@...il.com,
        corbet@....net, dave.hansen@...ux.intel.com, david@...hat.com,
        ebiederm@...ssion.com, hagen@...u.net, jack@...e.cz,
        Kees Cook <keescook@...omium.org>, kirill@...temov.name,
        kucharsk@...il.com, linkinjeon@...nel.org,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        Linux-MM <linux-mm@...ck.org>, longpeng2@...wei.com,
        luto@...nel.org, pcc@...gle.com, rppt@...nel.org,
        sieberf@...zon.com, sjpark@...zon.de,
        Suren Baghdasaryan <surenb@...gle.com>, tst@...oebel-theuer.de,
        yzaikin@...gle.com
Subject: Re: [PATCH v2 5/9] mm/mshare: Add vm flag for shared PTE

On 6/30/22 08:59, Mark Hemment wrote:
> On Wed, 29 Jun 2022 at 23:54, Khalid Aziz <khalid.aziz@...cle.com> wrote:
>>
>> Add a bit to vm_flags to indicate a vma shares PTEs with others. Add
>> a function to determine if a vma shares PTE by checking this flag.
>> This is to be used to find the shared page table entries on page fault
>> for vmas sharing PTE.
>>
>> Signed-off-by: Khalid Aziz <khalid.aziz@...cle.com>
>> Signed-off-by: Matthew Wilcox (Oracle) <willy@...radead.org>
>> ---
>>   include/linux/mm.h             | 8 ++++++++
>>   include/trace/events/mmflags.h | 3 ++-
>>   mm/internal.h                  | 5 +++++
>>   3 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index bc8f326be0ce..0ddc3057f73b 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -310,11 +310,13 @@ extern unsigned int kobjsize(const void *objp);
>>   #define VM_HIGH_ARCH_BIT_2     34      /* bit only usable on 64-bit architectures */
>>   #define VM_HIGH_ARCH_BIT_3     35      /* bit only usable on 64-bit architectures */
>>   #define VM_HIGH_ARCH_BIT_4     36      /* bit only usable on 64-bit architectures */
>> +#define VM_HIGH_ARCH_BIT_5     37      /* bit only usable on 64-bit architectures */
>>   #define VM_HIGH_ARCH_0 BIT(VM_HIGH_ARCH_BIT_0)
>>   #define VM_HIGH_ARCH_1 BIT(VM_HIGH_ARCH_BIT_1)
>>   #define VM_HIGH_ARCH_2 BIT(VM_HIGH_ARCH_BIT_2)
>>   #define VM_HIGH_ARCH_3 BIT(VM_HIGH_ARCH_BIT_3)
>>   #define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4)
>> +#define VM_HIGH_ARCH_5 BIT(VM_HIGH_ARCH_BIT_5)
>>   #endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */
>>
>>   #ifdef CONFIG_ARCH_HAS_PKEYS
>> @@ -356,6 +358,12 @@ extern unsigned int kobjsize(const void *objp);
>>   # define VM_MTE_ALLOWED        VM_NONE
>>   #endif
>>
>> +#ifdef CONFIG_ARCH_USES_HIGH_VMA_FLAGS
>> +#define VM_SHARED_PT   VM_HIGH_ARCH_5
>> +#else
>> +#define VM_SHARED_PT   0
>> +#endif
>> +
> 
> I'm not clear why mshare is using high-vma flags for VM_SHARED_PT.
> CONFIG_ARCH_USES_HIGH_VMA_FLAGS might not be defined, making mshare
> unsupported (or, rather, broken).
> Is this being done as there is a shortage of non-high flags?
> 0x00000800 is available, although it appears to be the last one (quick
> check).
> (When using the last 'normal' flag bit, good idea to highlight this in
> the cover letter.)

It indeed is because of shortage of non-high flag. 0x00000800 is the only non-high flag available and I am inclined to 
leave that last flag for more fundamental features. Then again if we want to move hugetlbfs page table sharing code over 
to mshare base code, it might be necessary to consume that last flag. Nevertheless, mshare code should not break if 
high-vma flags are not available. I definitely need to fix that.

Thanks,
Khalid

> 
>>   #ifndef VM_GROWSUP
>>   # define VM_GROWSUP    VM_NONE
>>   #endif
>> diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
>> index e87cb2b80ed3..30e56cbac99b 100644
>> --- a/include/trace/events/mmflags.h
>> +++ b/include/trace/events/mmflags.h
>> @@ -194,7 +194,8 @@ IF_HAVE_VM_SOFTDIRTY(VM_SOFTDIRTY,  "softdirty"     )               \
>>          {VM_MIXEDMAP,                   "mixedmap"      },              \
>>          {VM_HUGEPAGE,                   "hugepage"      },              \
>>          {VM_NOHUGEPAGE,                 "nohugepage"    },              \
>> -       {VM_MERGEABLE,                  "mergeable"     }               \
>> +       {VM_MERGEABLE,                  "mergeable"     },              \
>> +       {VM_SHARED_PT,                  "sharedpt"      }               \
>>
>>   #define show_vma_flags(flags)                                          \
>>          (flags) ? __print_flags(flags, "|",                             \
>> diff --git a/mm/internal.h b/mm/internal.h
>> index c0f8fbe0445b..3f2790aea918 100644
>> --- a/mm/internal.h
>> +++ b/mm/internal.h
>> @@ -861,4 +861,9 @@ struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags);
>>
>>   DECLARE_PER_CPU(struct per_cpu_nodestat, boot_nodestats);
>>
>> +static inline bool vma_is_shared(const struct vm_area_struct *vma)
>> +{
>> +       return vma->vm_flags & VM_SHARED_PT;
>> +}
>> +
>>   #endif /* __MM_INTERNAL_H */
>> --
>> 2.32.0
>>