lkml.org 
[lkml]   [2009]   [Sep]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: No more bits in vm_area_struct's vm_flags.
    On Mon, 28 Sep 2009 22:00:20 +0100 (BST)
    Hugh Dickins <hugh.dickins@tiscali.co.uk> wrote:

    >> On Tue, 29 Sep 2009, KAMEZAWA Hiroyuki wrote:
    > >
    > > Every time I see long long in the kernel, my concern is
    > > "do I need spinlock to access this for 32bit arch ? is it safe ?".
    > > (And it makes binary=>disassemble=>C (by eyes) a bit difficult)
    > > Then, I don't like long long personally.
    > >
    > > Another reason is some other calls like test_bit() cannot be used against
    > > long long. (even if it's not used _now_)
    > >
    > > Maybe vm->vm_flags will not require extra locks because
    > > it can be protected by bigger lock as mmap_sem.
    >
    > I think that even as you wrote, you guessed I wouldn't be persuaded ;)
    > It sounds like you've had a bad experience with a long long in the past.
    >
    yes ;)


    > We already have to have locking for vm_flags, of course we do: it's
    > mmap_sem, yes, though I think you'll find some exceptions which know
    > they have exclusive access without it.
    >
    > We use ordinary logical operations on vm_flags, we don't need it to
    > be atomic, we don't need an additional spinlock, we don't need to use
    > test_bit(). It's very easy! (But irritating in a few places which
    > have to down_write mmap_sem for no other reason than to update vm_flags.)
    >
    Okay, I'll have no objections.

    Just a notice from lines stripped by grep

    (1) using "int" will be never correct even on 32bit.
    ==
    vm_flags 242 arch/mips/mm/c-r3k.c int exec = vma->vm_flags & VM_EXEC;
    vm_flags 293 drivers/char/mem.c return vma->vm_flags & VM_MAYSHARE;
    vm_flags 44 mm/madvise.c int new_flags = vma->vm_flags;
    vm_flags 547 mm/memory.c unsigned long vm_flags = vma->vm_flags;

    But yes, it will be not a terrible bug for a while.

    (2) All vm macros should be defined with ULL suffix. for supporing ~
    ==
    vm_flags 30 arch/x86/mm/hugetlbpage.c unsigned long vm_flags = vma->vm_flags & ~VM_LOCKED;

    (3) vma_merge()'s vm_flags should be ULL.


    Not so many places as I thought..




    > > Then, please make it to be long long if its's recommended.
    > >
    > > keeping vm_flags to be 32bit may makes vma_merge() ugly.
    > > If so, long long is a choice.
    >
    > unsigned long long is certainly the natural choice: that way leaves
    > freedom for people to add more flags in future without worrying about
    > which flags variable to put them into. I'd better explain some of my
    > objections to Nigel's patch in a reply to him rather than here.
    >
    > I have made up a patch to convert it to unsigned long long (not gone
    > through all the arches yet though), mainly to try a build to see how
    > it works out in practice. I used a config which built most of the
    > non-debugging objects in mm/, things like migration and mempolicy
    > and memcg and ksm and so forth, but not kmemleak.
    >
    > And I have to admit that the 834 bytes it added to i386 kernel text
    > is more than I was expecting, more than I can just brush away as "in
    > the noise". I don't fully understand it yet. There's a few silly
    > "andl $0xffffffff"s from the compiler (4.3.2), but not enough to
    > worry about. Typically enlarged objects grow by 4 bytes, presumably
    > clearing the upper half when setting vma->vm_flags, fair enough.
    >
    > 300 bytes of growth is in mmap.o, 100 bytes of that in do_mmap_pgoff();
    > yet I don't see why it needed to grow by more than, say, 12 bytes.
    >
    > My current feeling is that unsigned long long is the right way to
    > go, but given the bloat, we shouldn't convert over until we need to:
    > right now we should look to save a few flags instead.

    Okay,

    Thanks,
    -Kame




    \
     
     \ /
      Last update: 2009-09-29 04:03    [W:0.048 / U:1.864 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site