From 8d218c8ec93eab537acc7dcd63ce1dd8983d79aa Mon Sep 17 00:00:00 2001 From: Sunil Khatri Date: Tue, 23 Jun 2015 19:37:27 +0530 Subject: [PATCH] msm: kgsl: Make global memory statistics atomic If you run aggressive memory operations long enough, eventually you will notice that the KGSL memory statistics will explode up into the 3G range. This is a race condition on the statistics math which ends up going negative in certain cases. Turning the statistics variables into atomics should solve the problem once and for all. ONCE AND FOR ALL. Change-Id: Ic0dedbad61762d667550c22190c4f9d0b453829b Signed-off-by: Jordan Crouse Signed-off-by: Sunil Khatri --- drivers/gpu/msm/kgsl.c | 16 +++++-- drivers/gpu/msm/kgsl.h | 27 +++++++----- drivers/gpu/msm/kgsl_gpummu.c | 8 ++-- drivers/gpu/msm/kgsl_mmu.c | 75 +++++++++++++++++--------------- drivers/gpu/msm/kgsl_mmu.h | 8 ++-- drivers/gpu/msm/kgsl_sharedmem.c | 68 +++++++++++------------------ 6 files changed, 101 insertions(+), 101 deletions(-) diff --git a/drivers/gpu/msm/kgsl.c b/drivers/gpu/msm/kgsl.c index a5b25a004a2..bdf2aa5c183 100644 --- a/drivers/gpu/msm/kgsl.c +++ b/drivers/gpu/msm/kgsl.c @@ -273,7 +273,8 @@ kgsl_mem_entry_destroy(struct kref *kref) kgsl_mem_entry_detach_process(entry); if (entry->memtype != KGSL_MEM_ENTRY_KERNEL) - kgsl_driver.stats.mapped -= entry->memdesc.size; + atomic_sub(entry->memdesc.size, + &kgsl_driver.stats.mapped); /* * Ion takes care of freeing the sglist for us so @@ -3080,8 +3081,8 @@ static long kgsl_ioctl_map_user_mem(struct kgsl_device_private *dev_priv, /* Adjust the returned value for a non 4k aligned offset */ param->gpuaddr = entry->memdesc.gpuaddr + (param->offset & ~PAGE_MASK); - KGSL_STATS_ADD(param->len, kgsl_driver.stats.mapped, - kgsl_driver.stats.mapped_max); + KGSL_STATS_ADD(param->len, &kgsl_driver.stats.mapped, + &kgsl_driver.stats.mapped_max); kgsl_process_add_stats(private, entry->memtype, param->len); @@ -4112,6 +4113,15 @@ struct kgsl_driver kgsl_driver = { * 8064 and 8974 once the region to be flushed is > 16mb. */ .full_cache_threshold = SZ_16M, + + .stats.vmalloc = ATOMIC_INIT(0), + .stats.vmalloc_max = ATOMIC_INIT(0), + .stats.page_alloc = ATOMIC_INIT(0), + .stats.page_alloc_max = ATOMIC_INIT(0), + .stats.coherent = ATOMIC_INIT(0), + .stats.coherent_max = ATOMIC_INIT(0), + .stats.mapped = ATOMIC_INIT(0), + .stats.mapped_max = ATOMIC_INIT(0), }; EXPORT_SYMBOL(kgsl_driver); diff --git a/drivers/gpu/msm/kgsl.h b/drivers/gpu/msm/kgsl.h index 2ee4c2c2a8d..83a187878a5 100644 --- a/drivers/gpu/msm/kgsl.h +++ b/drivers/gpu/msm/kgsl.h @@ -72,8 +72,14 @@ the statisic is greater then _max, set _max */ -#define KGSL_STATS_ADD(_size, _stat, _max) \ - do { _stat += (_size); if (_stat > _max) _max = _stat; } while (0) +static inline void KGSL_STATS_ADD(uint32_t size, atomic_t *stat, + atomic_t *max) +{ + uint32_t ret = atomic_add_return(size, stat); + + if (ret > atomic_read(max)) + atomic_set(max, ret); +} #define KGSL_MAX_NUMIBS 100000 @@ -106,15 +112,14 @@ struct kgsl_driver { void *ptpool; struct { - unsigned int vmalloc; - unsigned int vmalloc_max; - unsigned int page_alloc; - unsigned int page_alloc_max; - unsigned int coherent; - unsigned int coherent_max; - unsigned int mapped; - unsigned int mapped_max; - unsigned int histogram[16]; + atomic_t vmalloc; + atomic_t vmalloc_max; + atomic_t page_alloc; + atomic_t page_alloc_max; + atomic_t coherent; + atomic_t coherent_max; + atomic_t mapped; + atomic_t mapped_max; } stats; unsigned int full_cache_threshold; }; diff --git a/drivers/gpu/msm/kgsl_gpummu.c b/drivers/gpu/msm/kgsl_gpummu.c index 2634e4f05a2..2c3040f3623 100644 --- a/drivers/gpu/msm/kgsl_gpummu.c +++ b/drivers/gpu/msm/kgsl_gpummu.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2011-2013, The Linux Foundation. All rights reserved. +/* Copyright (c) 2011,2013-2015, 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 @@ -371,7 +371,7 @@ void kgsl_gpummu_destroy_pagetable(struct kgsl_pagetable *pt) kgsl_ptpool_free((struct kgsl_ptpool *)kgsl_driver.ptpool, gpummu_pt->base.hostptr); - kgsl_driver.stats.coherent -= KGSL_PAGETABLE_SIZE; + atomic_sub(KGSL_PAGETABLE_SIZE, &kgsl_driver.stats.coherent); kfree(gpummu_pt->tlbflushfilter.base); @@ -469,8 +469,8 @@ static void *kgsl_gpummu_create_pagetable(void) /* ptpool allocations are from coherent memory, so update the device statistics acordingly */ - KGSL_STATS_ADD(KGSL_PAGETABLE_SIZE, kgsl_driver.stats.coherent, - kgsl_driver.stats.coherent_max); + KGSL_STATS_ADD(KGSL_PAGETABLE_SIZE, &kgsl_driver.stats.coherent, + &kgsl_driver.stats.coherent_max); return (void *)gpummu_pt; diff --git a/drivers/gpu/msm/kgsl_mmu.c b/drivers/gpu/msm/kgsl_mmu.c index 45911653c5d..182f4e0c2b3 100755 --- a/drivers/gpu/msm/kgsl_mmu.c +++ b/drivers/gpu/msm/kgsl_mmu.c @@ -158,8 +158,11 @@ sysfs_show_entries(struct kobject *kobj, pt = _get_pt_from_kobj(kobj); - if (pt) - ret += snprintf(buf, PAGE_SIZE, "%d\n", pt->stats.entries); + if (pt) { + unsigned int val = atomic_read(&pt->stats.entries); + + ret += snprintf(buf, PAGE_SIZE, "%d\n", val); + } kgsl_put_pagetable(pt); return ret; @@ -175,8 +178,11 @@ sysfs_show_mapped(struct kobject *kobj, pt = _get_pt_from_kobj(kobj); - if (pt) - ret += snprintf(buf, PAGE_SIZE, "%d\n", pt->stats.mapped); + if (pt) { + unsigned int val = atomic_read(&pt->stats.mapped); + + ret += snprintf(buf, PAGE_SIZE, "%d\n", val); + } kgsl_put_pagetable(pt); return ret; @@ -211,8 +217,11 @@ sysfs_show_max_mapped(struct kobject *kobj, pt = _get_pt_from_kobj(kobj); - if (pt) - ret += snprintf(buf, PAGE_SIZE, "%d\n", pt->stats.max_mapped); + if (pt) { + unsigned int val = atomic_read(&pt->stats.max_mapped); + + ret += snprintf(buf, PAGE_SIZE, "%d\n", val); + } kgsl_put_pagetable(pt); return ret; @@ -228,8 +237,11 @@ sysfs_show_max_entries(struct kobject *kobj, pt = _get_pt_from_kobj(kobj); - if (pt) - ret += snprintf(buf, PAGE_SIZE, "%d\n", pt->stats.max_entries); + if (pt) { + unsigned int val = atomic_read(&pt->stats.max_entries); + + ret += snprintf(buf, PAGE_SIZE, "%d\n", val); + } kgsl_put_pagetable(pt); return ret; @@ -476,6 +488,11 @@ kgsl_mmu_createpagetableobject(struct kgsl_mmu *mmu, pagetable->max_entries = KGSL_PAGETABLE_ENTRIES(ptsize); pagetable->fault_addr = 0xFFFFFFFF; + atomic_set(&pagetable->stats.entries, 0); + atomic_set(&pagetable->stats.mapped, 0); + atomic_set(&pagetable->stats.max_mapped, 0); + atomic_set(&pagetable->stats.max_entries, 0); + /* * create a separate kgsl pool for IOMMU, global mappings can be mapped * just once from this pool of the defaultpagetable @@ -692,14 +709,16 @@ kgsl_mmu_get_gpuaddr(struct kgsl_pagetable *pagetable, memdesc->gpuaddr = gen_pool_alloc_aligned(pool, size, page_align); if (memdesc->gpuaddr == 0) { + unsigned int entries = atomic_read(&pagetable->stats.entries); + unsigned int mapped = atomic_read(&pagetable->stats.mapped); KGSL_CORE_ERR("gen_pool_alloc(%d) failed, pool: %s\n", size, (pool == pagetable->kgsl_pool) ? "kgsl_pool" : "general_pool"); KGSL_CORE_ERR(" [%d] allocated=%d, entries=%d\n", pagetable->name, - pagetable->stats.mapped, - pagetable->stats.entries); + mapped, + entries); return -ENOMEM; } } @@ -726,31 +745,19 @@ kgsl_mmu_map(struct kgsl_pagetable *pagetable, if (kgsl_memdesc_has_guard_page(memdesc)) size += PAGE_SIZE; - if (KGSL_MMU_TYPE_IOMMU != kgsl_mmu_get_mmutype()) - spin_lock(&pagetable->lock); ret = pagetable->pt_ops->mmu_map(pagetable, memdesc, protflags, &pagetable->tlb_flags); - if (KGSL_MMU_TYPE_IOMMU == kgsl_mmu_get_mmutype()) - spin_lock(&pagetable->lock); - if (ret) - goto done; + if (ret == 0) { + KGSL_STATS_ADD(size, &pagetable->stats.mapped, + &pagetable->stats.max_mapped); - /* Keep track of the statistics for the sysfs files */ + KGSL_STATS_ADD(size, &pagetable->stats.entries, + &pagetable->stats.max_entries); - KGSL_STATS_ADD(1, pagetable->stats.entries, - pagetable->stats.max_entries); + memdesc->priv |= KGSL_MEMDESC_MAPPED; + } - KGSL_STATS_ADD(size, pagetable->stats.mapped, - pagetable->stats.max_mapped); - - spin_unlock(&pagetable->lock); - memdesc->priv |= KGSL_MEMDESC_MAPPED; - - return 0; - -done: - spin_unlock(&pagetable->lock); return ret; } EXPORT_SYMBOL(kgsl_mmu_map); @@ -824,8 +831,6 @@ kgsl_mmu_unmap(struct kgsl_pagetable *pagetable, start_addr = memdesc->gpuaddr; end_addr = (memdesc->gpuaddr + size); - if (KGSL_MMU_TYPE_IOMMU != kgsl_mmu_get_mmutype()) - spin_lock(&pagetable->lock); pagetable->pt_ops->mmu_unmap(pagetable, memdesc, &pagetable->tlb_flags); @@ -834,15 +839,13 @@ kgsl_mmu_unmap(struct kgsl_pagetable *pagetable, (pagetable->fault_addr < end_addr)) pagetable->fault_addr = 0; - if (KGSL_MMU_TYPE_IOMMU == kgsl_mmu_get_mmutype()) - spin_lock(&pagetable->lock); /* Remove the statistics */ - pagetable->stats.entries--; - pagetable->stats.mapped -= size; + atomic_dec(&pagetable->stats.entries); + atomic_sub(size, &pagetable->stats.mapped); - spin_unlock(&pagetable->lock); if (!kgsl_memdesc_is_global(memdesc)) memdesc->priv &= ~KGSL_MEMDESC_MAPPED; + return 0; } EXPORT_SYMBOL(kgsl_mmu_unmap); diff --git a/drivers/gpu/msm/kgsl_mmu.h b/drivers/gpu/msm/kgsl_mmu.h index 5e3386a7c23..f15c6980128 100644 --- a/drivers/gpu/msm/kgsl_mmu.h +++ b/drivers/gpu/msm/kgsl_mmu.h @@ -114,10 +114,10 @@ struct kgsl_pagetable { struct kobject *kobj; struct { - unsigned int entries; - unsigned int mapped; - unsigned int max_mapped; - unsigned int max_entries; + atomic_t entries; + atomic_t mapped; + atomic_t max_mapped; + atomic_t max_entries; } stats; const struct kgsl_mmu_pt_ops *pt_ops; unsigned int tlb_flags; diff --git a/drivers/gpu/msm/kgsl_sharedmem.c b/drivers/gpu/msm/kgsl_sharedmem.c index 6b5a555a754..e46d8b46e9b 100644 --- a/drivers/gpu/msm/kgsl_sharedmem.c +++ b/drivers/gpu/msm/kgsl_sharedmem.c @@ -218,40 +218,25 @@ static int kgsl_drv_memstat_show(struct device *dev, unsigned int val = 0; if (!strncmp(attr->attr.name, "vmalloc", 7)) - val = kgsl_driver.stats.vmalloc; + val = atomic_read(&kgsl_driver.stats.vmalloc); else if (!strncmp(attr->attr.name, "vmalloc_max", 11)) - val = kgsl_driver.stats.vmalloc_max; + val = atomic_read(&kgsl_driver.stats.vmalloc_max); else if (!strncmp(attr->attr.name, "page_alloc", 10)) - val = kgsl_driver.stats.page_alloc; + val = atomic_read(&kgsl_driver.stats.page_alloc); else if (!strncmp(attr->attr.name, "page_alloc_max", 14)) - val = kgsl_driver.stats.page_alloc_max; + val = atomic_read(&kgsl_driver.stats.page_alloc_max); else if (!strncmp(attr->attr.name, "coherent", 8)) - val = kgsl_driver.stats.coherent; + val = atomic_read(&kgsl_driver.stats.coherent); else if (!strncmp(attr->attr.name, "coherent_max", 12)) - val = kgsl_driver.stats.coherent_max; + val = atomic_read(&kgsl_driver.stats.coherent_max); else if (!strncmp(attr->attr.name, "mapped", 6)) - val = kgsl_driver.stats.mapped; + val = atomic_read(&kgsl_driver.stats.mapped); else if (!strncmp(attr->attr.name, "mapped_max", 10)) - val = kgsl_driver.stats.mapped_max; + val = atomic_read(&kgsl_driver.stats.mapped_max); return snprintf(buf, PAGE_SIZE, "%u\n", val); } -static int kgsl_drv_histogram_show(struct device *dev, - struct device_attribute *attr, - char *buf) -{ - int len = 0; - int i; - - for (i = 0; i < 16; i++) - len += snprintf(buf + len, PAGE_SIZE - len, "%d ", - kgsl_driver.stats.histogram[i]); - - len += snprintf(buf + len, PAGE_SIZE - len, "\n"); - return len; -} - static int kgsl_drv_full_cache_threshold_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) @@ -283,7 +268,6 @@ DEVICE_ATTR(coherent, 0444, kgsl_drv_memstat_show, NULL); DEVICE_ATTR(coherent_max, 0444, kgsl_drv_memstat_show, NULL); DEVICE_ATTR(mapped, 0444, kgsl_drv_memstat_show, NULL); DEVICE_ATTR(mapped_max, 0444, kgsl_drv_memstat_show, NULL); -DEVICE_ATTR(histogram, 0444, kgsl_drv_histogram_show, NULL); DEVICE_ATTR(full_cache_threshold, 0644, kgsl_drv_full_cache_threshold_show, kgsl_drv_full_cache_threshold_store); @@ -297,7 +281,6 @@ static const struct device_attribute *drv_attr_list[] = { &dev_attr_coherent_max, &dev_attr_mapped, &dev_attr_mapped_max, - &dev_attr_histogram, &dev_attr_full_cache_threshold, NULL }; @@ -413,7 +396,8 @@ static void kgsl_page_alloc_unmap_kernel(struct kgsl_memdesc *memdesc) if (memdesc->hostptr_count) goto done; vunmap(memdesc->hostptr); - kgsl_driver.stats.vmalloc -= memdesc->size; + + atomic_sub(memdesc->size, &kgsl_driver.stats.vmalloc); memdesc->hostptr = NULL; done: mutex_unlock(&kernel_map_global_lock); @@ -425,7 +409,7 @@ static void kgsl_page_alloc_free(struct kgsl_memdesc *memdesc) struct scatterlist *sg; int sglen = memdesc->sglen; - kgsl_driver.stats.page_alloc -= memdesc->size; + atomic_sub(memdesc->size, &kgsl_driver.stats.page_alloc); kgsl_page_alloc_unmap_kernel(memdesc); @@ -484,8 +468,9 @@ static int kgsl_page_alloc_map_kernel(struct kgsl_memdesc *memdesc) memdesc->hostptr = vmap(pages, count, VM_IOREMAP, page_prot); if (memdesc->hostptr) - KGSL_STATS_ADD(memdesc->size, kgsl_driver.stats.vmalloc, - kgsl_driver.stats.vmalloc_max); + KGSL_STATS_ADD(memdesc->size, + &kgsl_driver.stats.vmalloc, + &kgsl_driver.stats.vmalloc_max); else ret = -ENOMEM; vfree(pages); @@ -539,7 +524,8 @@ done: static void kgsl_ebimem_free(struct kgsl_memdesc *memdesc) { - kgsl_driver.stats.coherent -= memdesc->size; + atomic_sub(memdesc->size, + &kgsl_driver.stats.coherent); kgsl_ebimem_unmap_kernel(memdesc); /* we certainly do not expect the hostptr to still be mapped */ BUG_ON(memdesc->hostptr); @@ -568,7 +554,8 @@ done: static void kgsl_coherent_free(struct kgsl_memdesc *memdesc) { - kgsl_driver.stats.coherent -= memdesc->size; + atomic_sub(memdesc->size, + &kgsl_driver.stats.coherent); dma_free_coherent(NULL, memdesc->size, memdesc->hostptr, memdesc->physaddr); } @@ -629,7 +616,7 @@ _kgsl_sharedmem_page_alloc(struct kgsl_memdesc *memdesc, struct kgsl_pagetable *pagetable, size_t size) { - int pcount = 0, order, ret = 0; + int pcount = 0, ret = 0; int j, len, page_size, sglen_alloc, sglen = 0; struct page **pages = NULL; pgprot_t page_prot = pgprot_writecombine(PAGE_KERNEL); @@ -789,14 +776,9 @@ _kgsl_sharedmem_page_alloc(struct kgsl_memdesc *memdesc, outer_cache_range_op_sg(memdesc->sg, memdesc->sglen, KGSL_CACHE_OP_FLUSH); - order = get_order(size); - - if (order < 16) - kgsl_driver.stats.histogram[order]++; - done: - KGSL_STATS_ADD(memdesc->size, kgsl_driver.stats.page_alloc, - kgsl_driver.stats.page_alloc_max); + KGSL_STATS_ADD(memdesc->size, &kgsl_driver.stats.page_alloc, + &kgsl_driver.stats.page_alloc_max); if ((memdesc->sglen_alloc * sizeof(struct page *)) > PAGE_SIZE) vfree(pages); @@ -868,8 +850,8 @@ kgsl_sharedmem_alloc_coherent(struct kgsl_memdesc *memdesc, size_t size) /* Record statistics */ - KGSL_STATS_ADD(size, kgsl_driver.stats.coherent, - kgsl_driver.stats.coherent_max); + KGSL_STATS_ADD(size, &kgsl_driver.stats.coherent, + &kgsl_driver.stats.coherent_max); err: if (result) @@ -920,8 +902,8 @@ _kgsl_sharedmem_ebimem(struct kgsl_memdesc *memdesc, if (result) goto err; - KGSL_STATS_ADD(size, kgsl_driver.stats.coherent, - kgsl_driver.stats.coherent_max); + KGSL_STATS_ADD(size, &kgsl_driver.stats.coherent, + &kgsl_driver.stats.coherent_max); err: if (result)