lkml.org 
[lkml]   [2009]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 04/10] NOMMU: Make VMAs per MM as for MMU-mode linux [ver #3]
    On Thu, 08 Jan 2009 12:54:07 +0000 David Howells <dhowells@redhat.com> wrote:

    > Make VMAs per mm_struct as for MMU-mode linux.
    >
    >
    > ...
    >
    > --- a/fs/proc/meminfo.c
    > +++ b/fs/proc/meminfo.c
    > @@ -74,6 +74,9 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
    > "LowTotal: %8lu kB\n"
    > "LowFree: %8lu kB\n"
    > #endif
    > +#ifndef CONFIG_MMU
    > + "MmapCopy: %8lu kB\n"
    > +#endif
    > "SwapTotal: %8lu kB\n"
    > "SwapFree: %8lu kB\n"
    > "Dirty: %8lu kB\n"
    > @@ -116,6 +119,9 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
    > K(i.totalram-i.totalhigh),
    > K(i.freeram-i.freehigh),
    > #endif
    > +#ifndef CONFIG_MMU
    > + K((unsigned long) atomic_read(&mmap_pages_allocated)),
    > +#endif

    This is slightly wrong, as atomic_read() returns "int". It will show
    weird results on 64-bit nommu machines which allocate more than 2G pages :)

    > K(i.totalswap),
    > K(i.freeswap),
    > K(global_page_state(NR_FILE_DIRTY)),
    >
    > ...
    >
    > +static int nommu_vma_show(struct seq_file *m, struct vm_area_struct *vma)
    > +{
    > + unsigned long ino = 0;
    > + struct file *file;
    > + dev_t dev = 0;
    > + int flags, len;
    > +
    > + flags = vma->vm_flags;
    > + file = vma->vm_file;
    > +
    > + if (file) {
    > + struct inode *inode = vma->vm_file->f_path.dentry->d_inode;
    > + dev = inode->i_sb->s_dev;
    > + ino = inode->i_ino;
    > + }
    > +
    > + seq_printf(m,
    > + "%08lx-%08lx %c%c%c%c %08lx %02x:%02x %lu %n",
    > + vma->vm_start,
    > + vma->vm_end,
    > + flags & VM_READ ? 'r' : '-',
    > + flags & VM_WRITE ? 'w' : '-',
    > + flags & VM_EXEC ? 'x' : '-',
    > + flags & VM_MAYSHARE ? flags & VM_SHARED ? 'S' : 's' : 'p',
    > + vma->vm_pgoff << PAGE_SHIFT,

    This will overflow at file offsets >4G?

    > + MAJOR(dev), MINOR(dev), ino, &len);
    > +
    > + if (file) {
    > + len = 25 + sizeof(void *) * 6 - len;
    > + if (len < 1)
    > + len = 1;
    > + seq_printf(m, "%*c", len, ' ');
    > + seq_path(m, &file->f_path, "");
    > + }
    > +
    > + seq_putc(m, '\n');
    > + return 0;
    > +}
    > +
    >
    > ...
    >
    > --- a/mm/mmap.c
    > +++ b/mm/mmap.c
    > @@ -2472,3 +2472,13 @@ void mm_drop_all_locks(struct mm_struct *mm)
    >
    > mutex_unlock(&mm_all_locks_mutex);
    > }
    > +
    > +/*
    > + * initialise the VMA slab
    > + */
    > +void __init mmap_init(void)
    > +{
    > + vm_area_cachep = kmem_cache_create("vm_area_struct",
    > + sizeof(struct vm_area_struct), 0,
    > + SLAB_PANIC, NULL);

    We have the KMEM_CACHE() macro for this.

    It was initially added because we'd been through a period of changing
    the kmem_cache_create() arguments every ten minutes and we got tired of
    patching zillions of callsites.

    (I see this was copy-n-pasted from fork.c. Why was it moved?)
    >
    > ...
    >
    > + vm_region_jar = kmem_cache_create("vm_region_jar",
    > + sizeof(struct vm_region), 0,
    > + SLAB_PANIC, NULL);
    > + vm_area_cachep = kmem_cache_create("vm_area_struct",
    > + sizeof(struct vm_area_struct), 0,
    > + SLAB_PANIC, NULL);

    dittoes

    > -#endif /* DEBUG */
    >
    > /*
    > - * add a VMA into a process's mm_struct in the appropriate place in the list
    > - * - should be called with mm->mmap_sem held writelocked
    > + * validate the region tree
    > + * - the caller must hold the region lock
    > */
    > -static void add_vma_to_mm(struct mm_struct *mm, struct vm_list_struct *vml)
    > +#ifdef CONFIG_DEBUG_NOMMU_REGIONS
    > +static noinline void validate_nommu_regions(void)
    > {
    > - struct vm_list_struct **ppv;
    > + struct vm_region *region, *last;
    > + struct rb_node *p, *lastp;
    >
    > - for (ppv = &current->mm->context.vmlist; *ppv; ppv = &(*ppv)->next)
    > - if ((*ppv)->vma->vm_start > vml->vma->vm_start)
    > - break;
    > + lastp = rb_first(&nommu_region_tree);
    > + if (!lastp)
    > + return;
    > +
    > + last = rb_entry(lastp, struct vm_region, vm_rb);
    > + if (unlikely(last->vm_end <= last->vm_start))
    > + BUG();

    open-coded BUG_ON()

    > + while ((p = rb_next(lastp))) {
    > + region = rb_entry(p, struct vm_region, vm_rb);

    `region' could be local to this block, if one feels so inclined.

    > + last = rb_entry(lastp, struct vm_region, vm_rb);
    > +
    > + if (unlikely(region->vm_end <= region->vm_start))
    > + BUG();
    > + if (unlikely(region->vm_start < last->vm_end))
    > + BUG();

    BUG_ON()

    > - vml->next = *ppv;
    > - *ppv = vml;
    > + lastp = p;
    > + }
    > }
    > +#else
    > +#define validate_nommu_regions() do {} while(0)

    There's no need to implement this in cpp...

    >
    > ...
    >
    > -static inline struct vm_area_struct *find_vma_exact(struct mm_struct *mm,
    > - unsigned long addr)
    > +static void free_page_series(unsigned long from, unsigned long to)
    > {
    > - struct vm_list_struct *vml;
    > -
    > - /* search the vm_start ordered list */
    > - for (vml = mm->context.vmlist; vml; vml = vml->next) {
    > - if (vml->vma->vm_start == addr)
    > - return vml->vma;
    > - if (vml->vma->vm_start > addr)
    > - break;
    > + for (; from < to; from += PAGE_SIZE) {
    > + struct page *page = virt_to_page(from);
    > +
    > + kdebug("- free %lx", from);
    > + atomic_dec(&mmap_pages_allocated);

    This isn't counting "pages allocated". It's counting page *refcounts*.
    Now, perhaps these pages will always have a refcount of 1 (why?), in
    which case things happen to work out. But if so, the next line isn't
    needed:

    > + if (page_count(page) != 1)

    should be "== 1", surely?

    > + kdebug("free page %p [%d]", page, page_count(page));
    > + put_page(page);
    > }
    > -
    > - return NULL;
    > }
    >
    > /*
    > - * find a VMA in the global tree
    > + * release a reference to a region
    > + * - the caller must hold the region semaphore, which this releases

    "for writing"

    >
    > ...
    >
    > +int do_munmap(struct mm_struct *mm, unsigned long start, size_t len)
    > +{
    > + struct vm_area_struct *vma;
    > + struct rb_node *rb;
    > + unsigned long end = start + len;
    > + int ret;
    >
    > - update_hiwater_vm(mm);
    > - mm->total_vm -= len >> PAGE_SHIFT;
    > + kenter(",%lx,%zx", start, len);
    >
    > -#ifdef DEBUG
    > - show_process_blocks();
    > -#endif
    > + if (len == 0)
    > + return -EINVAL;
    > +
    > + /* find the first potentially overlapping VMA */
    > + vma = find_vma(mm, start);
    > + if (!vma) {
    > + printk(KERN_WARNING
    > + "munmap of memory not mmapped by process %d (%s):"
    > + " 0x%lx-0x%lx\n",
    > + current->pid, current->comm, start, start + len - 1);

    Unprivileged userspace can spam logs? Perhaps not a concern on typical
    nommu systems. But buggy userspace can spam logs, too, which _is_ a
    problem?

    > + return -EINVAL;
    > + }
    >
    > + /* we're allowed to split an anonymous VMA but not a file-backed one */
    > + if (vma->vm_file) {
    > + do {
    > + if (start > vma->vm_start) {
    > + kleave(" = -EINVAL [miss]");
    > + return -EINVAL;
    > + }
    > + if (end == vma->vm_end)
    > + goto erase_whole_vma;
    > + rb = rb_next(&vma->vm_rb);
    > + vma = rb_entry(rb, struct vm_area_struct, vm_rb);
    > + } while (rb);
    > + kleave(" = -EINVAL [split file]");
    > + return -EINVAL;
    > + } else {
    > + /* the chunk must be a subset of the VMA found */
    > + if (start == vma->vm_start && end == vma->vm_end)
    > + goto erase_whole_vma;
    > + if (start < vma->vm_start || end > vma->vm_end) {
    > + kleave(" = -EINVAL [superset]");
    > + return -EINVAL;
    > + }
    > + if (start & ~PAGE_MASK) {
    > + kleave(" = -EINVAL [unaligned start]");
    > + return -EINVAL;
    > + }
    > + if (end != vma->vm_end && end & ~PAGE_MASK) {
    > + kleave(" = -EINVAL [unaligned split]");
    > + return -EINVAL;
    > + }
    > + if (start != vma->vm_start && end != vma->vm_end) {
    > + ret = split_vma(mm, vma, start, 1);
    > + if (ret < 0) {
    > + kleave(" = %d [split]", ret);
    > + return ret;
    > + }
    > + }
    > + return shrink_vma(mm, vma, start, end);
    > + }
    > +
    > +erase_whole_vma:
    > + delete_vma_from_mm(vma);
    > + delete_vma(mm, vma);
    > + kleave(" = 0");
    > return 0;
    > }
    >
    > ...
    >



    \
     
     \ /
      Last update: 2009-01-13 09:01    [W:0.039 / U:122.572 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site