lkml.org 
[lkml]   [2011]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: mm: convert vma->vm_flags to 64bit
    On Tue, 12 Apr 2011 15:10:56 +0900 (JST) KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

    >
    > Benjamin, Hugh, I hope to add your S-O-B to this one because you are original author.
    > Can I do?
    >
    > Paul, Russell, This patch modifies arm and sh code a bit. I don't think
    > they are risky change. but I'm really glad if you see it.
    >
    >
    > Note: I confirmed x86, power and nommu-arm cross compiler build and
    > I've got no warning/error.
    >
    >
    >
    > >From d5a0d1c265e4caccb9ff5978c615f74019b65453 Mon Sep 17 00:00:00 2001
    > From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
    > Date: Tue, 12 Apr 2011 14:00:42 +0900
    > Subject: [PATCH] mm: convert vma->vm_flags to 64bit
    >
    > For years, powerpc people repeatedly request us to convert vm_flags
    > to 64bit. Because now it has no room to store an addional powerpc
    > specific flags.
    >
    > Here is previous discussion logs.
    >
    > http://lkml.org/lkml/2009/10/1/202
    > http://lkml.org/lkml/2010/4/27/23
    >
    > But, unforunately they didn't get merged. This is 3rd trial.
    > I've merged previous two posted patches and adapted it for
    > latest tree.
    >
    > Of cource, this patch has no functional change.

    That's a bit sad, but all 32 bits are used up, so we'll presumably need
    this change pretty soon anyway.

    How the heck did we end up using 32 flags??

    > @@ -217,7 +217,7 @@ vivt_flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned
    > {
    > if (cpumask_test_cpu(smp_processor_id(), mm_cpumask(vma->vm_mm)))
    > __cpuc_flush_user_range(start & PAGE_MASK, PAGE_ALIGN(end),
    > - vma->vm_flags);
    > + (unsigned long)vma->vm_flags);
    > }

    I'm surprised this change (and similar) are needed?

    Is it risky? What happens if we add yet another vm_flags bit and
    __cpuc_flush_user_range() wants to use it? I guess when that happens,
    __cpuc_flush_user_range() needs to be changed to take a ull.

    > -static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot)
    > +static inline unsigned long long arch_calc_vm_prot_bits(unsigned long prot)
    > {
    > - return (prot & PROT_SAO) ? VM_SAO : 0;
    > + return (prot & PROT_SAO) ? VM_SAO : 0ULL;
    > }

    The patch does a lot of adding "ULL" like this. But I don't think any
    of it is needed - won't the compiler do the right thing, without
    warning?

    > --- a/arch/x86/mm/hugetlbpage.c
    > +++ b/arch/x86/mm/hugetlbpage.c
    > @@ -26,8 +26,8 @@ static unsigned long page_table_shareable(struct vm_area_struct *svma,
    > unsigned long s_end = sbase + PUD_SIZE;
    >
    > /* Allow segments to share if only one is marked locked */
    > - unsigned long vm_flags = vma->vm_flags & ~VM_LOCKED;
    > - unsigned long svm_flags = svma->vm_flags & ~VM_LOCKED;
    > + unsigned long long vm_flags = vma->vm_flags & ~VM_LOCKED;
    > + unsigned long long svm_flags = svma->vm_flags & ~VM_LOCKED;

    hm, on second thoughts it is safer to add the ULL when we're doing ~ on
    the constants.

    > static inline int private_mapping_ok(struct vm_area_struct *vma)
    > {
    > - return vma->vm_flags & VM_MAYSHARE;
    > + return !!(vma->vm_flags & VM_MAYSHARE);
    > }

    Fair enough.

    > @@ -67,55 +67,55 @@ extern unsigned int kobjsize(const void *objp);
    > /*
    > * vm_flags in vm_area_struct, see mm_types.h.
    > */
    > -#define VM_READ 0x00000001 /* currently active flags */
    > -#define VM_WRITE 0x00000002
    > -#define VM_EXEC 0x00000004
    > -#define VM_SHARED 0x00000008
    > +#define VM_READ 0x00000001ULL /* currently active flags */
    > +#define VM_WRITE 0x00000002ULL
    > +#define VM_EXEC 0x00000004ULL
    > +#define VM_SHARED 0x00000008ULL
    >
    > /* mprotect() hardcodes VM_MAYREAD >> 4 == VM_READ, and so for r/w/x bits. */
    > -#define VM_MAYREAD 0x00000010 /* limits for mprotect() etc */
    > -#define VM_MAYWRITE 0x00000020
    > -#define VM_MAYEXEC 0x00000040
    > -#define VM_MAYSHARE 0x00000080
    > +#define VM_MAYREAD 0x00000010ULL /* limits for mprotect() etc */
    > +#define VM_MAYWRITE 0x00000020ULL
    > +#define VM_MAYEXEC 0x00000040ULL
    > +#define VM_MAYSHARE 0x00000080ULL
    >
    > -#define VM_GROWSDOWN 0x00000100 /* general info on the segment */
    > +#define VM_GROWSDOWN 0x00000100ULL /* general info on the segment */
    > #if defined(CONFIG_STACK_GROWSUP) || defined(CONFIG_IA64)
    > -#define VM_GROWSUP 0x00000200
    > +#define VM_GROWSUP 0x00000200ULL
    > #else
    > -#define VM_GROWSUP 0x00000000
    > -#define VM_NOHUGEPAGE 0x00000200 /* MADV_NOHUGEPAGE marked this vma */
    > +#define VM_GROWSUP 0x00000000ULL
    > +#define VM_NOHUGEPAGE 0x00000200ULL /* MADV_NOHUGEPAGE marked this vma */
    > #endif
    > -#define VM_PFNMAP 0x00000400 /* Page-ranges managed without "struct page", just pure PFN */
    > -#define VM_DENYWRITE 0x00000800 /* ETXTBSY on write attempts.. */
    > +#define VM_PFNMAP 0x00000400ULL /* Page-ranges managed without "struct page", just pure PFN */
    > +#define VM_DENYWRITE 0x00000800ULL /* ETXTBSY on write attempts.. */
    >
    > -#define VM_EXECUTABLE 0x00001000
    > -#define VM_LOCKED 0x00002000
    > -#define VM_IO 0x00004000 /* Memory mapped I/O or similar */
    > +#define VM_EXECUTABLE 0x00001000ULL
    > +#define VM_LOCKED 0x00002000ULL
    > +#define VM_IO 0x00004000ULL /* Memory mapped I/O or similar */

    A problem with this patch is that there might be unconverted code,
    either in-tree or out-of-tree or soon-to-be-in-tree. If that code
    isn't 64-bit aware then we'll be adding subtle bugs which take a long
    time to discover.

    One way to detect those bugs nice and quickly might be to change some
    of all of these existing constants so they use the upper 32-bits. But that
    will make __cpuc_flush_user_range() fail ;)



    \
     
     \ /
      Last update: 2011-04-12 08:33    [W:0.032 / U:31.792 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site