From 918ff3fbdfa770369fbaaaf67a1b079956e268b4 Mon Sep 17 00:00:00 2001 From: Jordan Crouse Date: Mon, 28 Dec 2020 16:06:42 +0530 Subject: [PATCH] msm: kgsl: Stop using memdesc->usermem It shouldn't be illegal for memory descriptors to have multiple virtual memory mappings under certain circumstances. If that is the case tracking a single usermem address for each memdesc no longer makes much sense. Get rid of the memdesc->usermem member and use an atomic counter to track mappings instead. Change-Id: Ic0dedbad31bafcd1019ccc8e68657cb7e3c72727 Signed-off-by: Jordan Crouse Signed-off-by: Harshitha Sai Neelati Signed-off-by: Pranav Patel --- drivers/gpu/msm/kgsl.c | 64 ++++++++++++++++++-------------- drivers/gpu/msm/kgsl.h | 11 +++--- drivers/gpu/msm/kgsl_debugfs.c | 22 +++++++---- drivers/gpu/msm/kgsl_sharedmem.c | 6 +-- drivers/gpu/msm/kgsl_trace.h | 8 ++-- 5 files changed, 62 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/msm/kgsl.c b/drivers/gpu/msm/kgsl.c index 6053d29ee013..28ae06da783c 100644 --- a/drivers/gpu/msm/kgsl.c +++ b/drivers/gpu/msm/kgsl.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2008-2020, The Linux Foundation. All rights reserved. +/* Copyright (c) 2008-2021, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -265,6 +265,7 @@ kgsl_mem_entry_create(void) kref_get(&entry->refcount); } + atomic_set(&entry->map_count, 0); return entry; } #ifdef CONFIG_DMA_SHARED_BUFFER @@ -459,9 +460,6 @@ static void kgsl_mem_entry_detach_process(struct kgsl_mem_entry *entry) type = kgsl_memdesc_usermem_type(&entry->memdesc); entry->priv->stats[type].cur -= entry->memdesc.size; - if (type != KGSL_MEM_ENTRY_ION) - entry->priv->gpumem_mapped -= entry->memdesc.mapsize; - spin_unlock(&entry->priv->mem_lock); kgsl_mmu_put_gpuaddr(&entry->memdesc); @@ -2129,7 +2127,7 @@ static int check_vma(unsigned long hostptr, u64 size) return true; } -static int memdesc_sg_virt(struct kgsl_memdesc *memdesc) +static int memdesc_sg_virt(struct kgsl_memdesc *memdesc, unsigned long useraddr) { int ret = 0; long npages = 0, i; @@ -2152,13 +2150,13 @@ static int memdesc_sg_virt(struct kgsl_memdesc *memdesc) } down_read(¤t->mm->mmap_sem); - if (!check_vma(memdesc->useraddr, memdesc->size)) { + if (!check_vma(useraddr, memdesc->size)) { up_read(¤t->mm->mmap_sem); ret = -EFAULT; goto out; } - npages = get_user_pages(memdesc->useraddr, sglen, write, pages, NULL); + npages = get_user_pages(useraddr, sglen, write, pages, NULL); up_read(¤t->mm->mmap_sem); ret = (npages < 0) ? (int)npages : 0; @@ -2196,7 +2194,6 @@ static int kgsl_setup_anon_useraddr(struct kgsl_pagetable *pagetable, entry->memdesc.pagetable = pagetable; entry->memdesc.size = (uint64_t) size; - entry->memdesc.useraddr = hostptr; entry->memdesc.flags |= (uint64_t)KGSL_MEMFLAGS_USERMEM_ADDR; if (kgsl_memdesc_use_cpu_map(&entry->memdesc)) { @@ -2204,15 +2201,15 @@ static int kgsl_setup_anon_useraddr(struct kgsl_pagetable *pagetable, /* Register the address in the database */ ret = kgsl_mmu_set_svm_region(pagetable, - (uint64_t) entry->memdesc.useraddr, (uint64_t) size); + (uint64_t) hostptr, (uint64_t) size); if (ret) return ret; - entry->memdesc.gpuaddr = (uint64_t) entry->memdesc.useraddr; + entry->memdesc.gpuaddr = (uint64_t) hostptr; } - return memdesc_sg_virt(&entry->memdesc); + return memdesc_sg_virt(&entry->memdesc, hostptr); } #ifdef CONFIG_DMA_SHARED_BUFFER @@ -2297,8 +2294,7 @@ static int kgsl_setup_dmabuf_useraddr(struct kgsl_device *device, return ret; } - /* Setup the user addr/cache mode for cache operations */ - entry->memdesc.useraddr = hostptr; + /* Setup the cache mode for cache operations */ _setup_cache_mode(entry, vma); up_read(¤t->mm->mmap_sem); return 0; @@ -3311,7 +3307,12 @@ long kgsl_ioctl_gpumem_get_info(struct kgsl_device_private *dev_priv, param->flags = (unsigned int) entry->memdesc.flags; param->size = (size_t) entry->memdesc.size; param->mmapsize = (size_t) kgsl_memdesc_footprint(&entry->memdesc); - param->useraddr = entry->memdesc.useraddr; + /* + * Entries can have multiple user mappings so thre isn't any one address + * we can report. Plus, the user should already know their mappings, so + * there isn't any value in reporting it back to them. + */ + param->useraddr = 0; kgsl_mem_entry_put(entry); return result; @@ -3780,9 +3781,6 @@ static int _sparse_bind(struct kgsl_process_private *process, if (memdesc->gpuaddr) return -EINVAL; - if (memdesc->useraddr != 0) - return -EINVAL; - pagetable = memdesc->pagetable; /* Clear out any mappings */ @@ -4062,7 +4060,12 @@ long kgsl_ioctl_gpuobj_info(struct kgsl_device_private *dev_priv, param->flags = entry->memdesc.flags; param->size = entry->memdesc.size; param->va_len = kgsl_memdesc_footprint(&entry->memdesc); - param->va_addr = (uint64_t) entry->memdesc.useraddr; + /* + * Entries can have multiple user mappings so thre isn't any one address + * we can report. Plus, the user should already know their mappings, so + * there isn't any value in reporting it back to them. + */ + param->va_addr = 0; kgsl_mem_entry_put(entry); return 0; @@ -4169,24 +4172,21 @@ static void kgsl_gpumem_vm_open(struct vm_area_struct *vma) if (kgsl_mem_entry_get(entry) == 0) vma->vm_private_data = NULL; + + atomic_inc(&entry->map_count); } static int kgsl_gpumem_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf) { struct kgsl_mem_entry *entry = vma->vm_private_data; - int ret; if (!entry) return VM_FAULT_SIGBUS; if (!entry->memdesc.ops || !entry->memdesc.ops->vmfault) return VM_FAULT_SIGBUS; - ret = entry->memdesc.ops->vmfault(&entry->memdesc, vma, vmf); - if ((ret == 0) || (ret == VM_FAULT_NOPAGE)) - entry->priv->gpumem_mapped += PAGE_SIZE; - - return ret; + return entry->memdesc.ops->vmfault(&entry->memdesc, vma, vmf); } static void @@ -4197,7 +4197,13 @@ kgsl_gpumem_vm_close(struct vm_area_struct *vma) if (!entry) return; - entry->memdesc.useraddr = 0; + /* + * Remove the memdesc from the mapped stat once all the mappings have + * gone away + */ + if (!atomic_dec_return(&entry->map_count)) + entry->priv->gpumem_mapped -= entry->memdesc.size; + kgsl_mem_entry_put(entry); } @@ -4236,7 +4242,8 @@ get_mmap_entry(struct kgsl_process_private *private, } } - if (entry->memdesc.useraddr != 0) { + /* Don't allow ourselves to remap user memory */ + if (entry->memdesc.flags & KGSL_MEMFLAGS_USERMEM_ADDR) { ret = -EBUSY; goto err_put; } @@ -4563,9 +4570,10 @@ static int kgsl_mmap(struct file *file, struct vm_area_struct *vma) vma->vm_file = file; - entry->memdesc.useraddr = vma->vm_start; + if (atomic_inc_return(&entry->map_count) == 1) + entry->priv->gpumem_mapped += entry->memdesc.size; - trace_kgsl_mem_mmap(entry); + trace_kgsl_mem_mmap(entry, vma->vm_start); return 0; } diff --git a/drivers/gpu/msm/kgsl.h b/drivers/gpu/msm/kgsl.h index 87bdde8b3924..41dbe2c0e4b1 100644 --- a/drivers/gpu/msm/kgsl.h +++ b/drivers/gpu/msm/kgsl.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2008-2020, The Linux Foundation. All rights reserved. +/* Copyright (c) 2008-2021, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -196,11 +196,9 @@ struct kgsl_memdesc_ops { * @pagetable: Pointer to the pagetable that the object is mapped in * @hostptr: Kernel virtual address * @hostptr_count: Number of threads using hostptr - * @useraddr: User virtual address (if applicable) * @gpuaddr: GPU virtual address * @physaddr: Physical address of the memory object * @size: Size of the memory object - * @mapsize: Size of memory mapped in userspace * @priv: Internal flags and settings * @sgt: Scatter gather table for allocated pages * @ops: Function hooks for the memdesc memory type @@ -215,11 +213,9 @@ struct kgsl_memdesc { struct kgsl_pagetable *pagetable; void *hostptr; unsigned int hostptr_count; - unsigned long useraddr; uint64_t gpuaddr; phys_addr_t physaddr; uint64_t size; - uint64_t mapsize; unsigned int priv; struct sg_table *sgt; struct kgsl_memdesc_ops *ops; @@ -277,6 +273,11 @@ struct kgsl_mem_entry { struct work_struct work; spinlock_t bind_lock; struct rb_root bind_tree; + /* + * @map_count: Count how many vmas this object is mapped in - used for + * debugfs accounting + */ + atomic_t map_count; }; struct kgsl_device_private; diff --git a/drivers/gpu/msm/kgsl_debugfs.c b/drivers/gpu/msm/kgsl_debugfs.c index ead45852f28f..2a6e60883637 100644 --- a/drivers/gpu/msm/kgsl_debugfs.c +++ b/drivers/gpu/msm/kgsl_debugfs.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2002,2008-2017,2020, The Linux Foundation. All rights reserved. +/* Copyright (c) 2002,2008-2017,2020-2021, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -152,7 +152,11 @@ static int print_mem_entry(void *data, void *ptr) flags[3] = get_alignflag(m); flags[4] = get_cacheflag(m); flags[5] = kgsl_memdesc_use_cpu_map(m) ? 'p' : '-'; - flags[6] = (m->useraddr) ? 'Y' : 'N'; + /* + * Show Y if at least one vma has this entry + * mapped (could be multiple) + */ + flags[6] = atomic_read(&entry->map_count) ? 'Y' : 'N'; flags[7] = kgsl_memdesc_is_secured(m) ? 's' : '-'; flags[8] = m->flags & KGSL_MEMFLAGS_SPARSE_PHYS ? 'P' : '-'; flags[9] = '\0'; @@ -163,12 +167,16 @@ static int print_mem_entry(void *data, void *ptr) kgsl_get_egl_counts(entry, &egl_surface_count, &egl_image_count); - seq_printf(s, "%pK %pK %16llu %5d %9s %10s %16s %5d %16llu %6d %6d", + seq_printf(s, "%pK %d %16llu %5d %9s %10s %16s %5d %16d %6d %6d", (uint64_t *)(uintptr_t) m->gpuaddr, - (unsigned long *) m->useraddr, - m->size, entry->id, flags, + /* + * Show zero for the useraddr - we can't reliably track + * that value for multiple vmas anyway + */ + 0, m->size, entry->id, flags, memtype_str(usermem_type), - usage, (m->sgt ? m->sgt->nents : 0), m->mapsize, + usage, (m->sgt ? m->sgt->nents : 0), + atomic_read(&entry->map_count), egl_surface_count, egl_image_count); if (entry->metadata[0] != 0) @@ -239,7 +247,7 @@ static int process_mem_seq_show(struct seq_file *s, void *ptr) if (ptr == SEQ_START_TOKEN) { seq_printf(s, "%16s %16s %16s %5s %9s %10s %16s %5s %16s %6s %6s\n", "gpuaddr", "useraddr", "size", "id", "flags", "type", - "usage", "sglen", "mapsize", "eglsrf", "eglimg"); + "usage", "sglen", "mapcount", "eglsrf", "eglimg"); return 0; } else return print_mem_entry(s, ptr); diff --git a/drivers/gpu/msm/kgsl_sharedmem.c b/drivers/gpu/msm/kgsl_sharedmem.c index 498600dec691..37491532ffd8 100644 --- a/drivers/gpu/msm/kgsl_sharedmem.c +++ b/drivers/gpu/msm/kgsl_sharedmem.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2002,2007-2020, The Linux Foundation. All rights reserved. +/* Copyright (c) 2002,2007-2021, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -445,8 +445,6 @@ static int kgsl_page_alloc_vmfault(struct kgsl_memdesc *memdesc, get_page(page); vmf->page = page; - memdesc->mapsize += PAGE_SIZE; - return 0; } @@ -577,8 +575,6 @@ static int kgsl_contiguous_vmfault(struct kgsl_memdesc *memdesc, else if (ret == -EFAULT) return VM_FAULT_SIGBUS; - memdesc->mapsize += PAGE_SIZE; - return VM_FAULT_NOPAGE; } diff --git a/drivers/gpu/msm/kgsl_trace.h b/drivers/gpu/msm/kgsl_trace.h index 478fc5feddac..b40a16122e86 100644 --- a/drivers/gpu/msm/kgsl_trace.h +++ b/drivers/gpu/msm/kgsl_trace.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2011-2017,2020, The Linux Foundation. All rights reserved. +/* Copyright (c) 2011-2017,2020-2021, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -432,9 +432,9 @@ TRACE_EVENT(kgsl_mem_alloc, TRACE_EVENT(kgsl_mem_mmap, - TP_PROTO(struct kgsl_mem_entry *mem_entry), + TP_PROTO(struct kgsl_mem_entry *mem_entry, unsigned long useraddr), - TP_ARGS(mem_entry), + TP_ARGS(mem_entry, useraddr), TP_STRUCT__entry( __field(unsigned long, useraddr) @@ -446,7 +446,7 @@ TRACE_EVENT(kgsl_mem_mmap, ), TP_fast_assign( - __entry->useraddr = mem_entry->memdesc.useraddr; + __entry->useraddr = useraddr; __entry->gpuaddr = mem_entry->memdesc.gpuaddr; __entry->size = mem_entry->memdesc.size; kgsl_get_memory_usage(__entry->usage, sizeof(__entry->usage),