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

Khalid Aziz
2022-06-30T15:48:54Z

On 6/30/22 08:59, Mark Hemment wrote:
> On Wed, 29 Jun 2022 at 23:54, Khalid Aziz <khalid.aziz@oracle.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@oracle.com>
>> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.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
>>