17.06.2019 16:33, Hans Verkuil пишет:
> On 6/2/19 11:37 PM, Dmitry Osipenko wrote:
>> Frequent IOMMU remappings take about 50% of CPU usage because there is
>> quite a lot to remap. Defer dmabuf's unmapping by 5 seconds in order to
>> mitigate the mapping overhead which goes away completely and driver works
>> as fast as in a case of a disabled IOMMU. The case of a disabled IOMMU
>> should also benefit a tad from the caching since CPU cache maintenance
>> that happens on dmabuf's attaching takes some resources.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
>> ---
>> drivers/staging/media/tegra-vde/Makefile | 2 +-
>> .../staging/media/tegra-vde/dmabuf-cache.c | 223 ++++++++++++++++++
>> drivers/staging/media/tegra-vde/iommu.c | 2 -
>> drivers/staging/media/tegra-vde/vde.c | 143 +++--------
>> drivers/staging/media/tegra-vde/vde.h | 18 +-
>> 5 files changed, 273 insertions(+), 115 deletions(-)
>> create mode 100644 drivers/staging/media/tegra-vde/dmabuf-cache.c
>>
>> diff --git a/drivers/staging/media/tegra-vde/Makefile b/drivers/staging/media/tegra-vde/Makefile
>> index c11867e28233..2827f7601de8 100644
>> --- a/drivers/staging/media/tegra-vde/Makefile
>> +++ b/drivers/staging/media/tegra-vde/Makefile
>> @@ -1,3 +1,3 @@
>> # SPDX-License-Identifier: GPL-2.0
>> -tegra-vde-y := vde.o iommu.o
>> +tegra-vde-y := vde.o iommu.o dmabuf-cache.o
>> obj-$(CONFIG_TEGRA_VDE) += tegra-vde.o
>> diff --git a/drivers/staging/media/tegra-vde/dmabuf-cache.c b/drivers/staging/media/tegra-vde/dmabuf-cache.c
>> new file mode 100644
>> index 000000000000..fcde8d1c37e7
>> --- /dev/null
>> +++ b/drivers/staging/media/tegra-vde/dmabuf-cache.c
>> @@ -0,0 +1,223 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * NVIDIA Tegra Video decoder driver
>> + *
>> + * Copyright (C) 2016-2019 GRATE-DRIVER project
>> + */
>> +
>> +#include <linux/dma-buf.h>
>> +#include <linux/iova.h>
>> +#include <linux/kernel.h>
>> +#include <linux/list.h>
>> +#include <linux/sched.h>
>> +#include <linux/slab.h>
>> +#include <linux/workqueue.h>
>> +
>> +#include "vde.h"
>> +
>> +struct tegra_vde_cache_entry {
>> + enum dma_data_direction dma_dir;
>> + struct dma_buf_attachment *a;
>> + struct delayed_work dwork;
>> + struct tegra_vde *vde;
>> + struct list_head list;
>> + struct sg_table *sgt;
>> + struct iova *iova;
>> + unsigned int refcnt;
>> +};
>> +
>> +static void tegra_vde_release_entry(struct tegra_vde_cache_entry *entry)
>> +{
>> + struct dma_buf *dmabuf = entry->a->dmabuf;
>> +
>> + WARN_ON_ONCE(entry->refcnt);
>> +
>> + if (entry->vde->domain)
>> + tegra_vde_iommu_unmap(entry->vde, entry->iova);
>> +
>> + dma_buf_unmap_attachment(entry->a, entry->sgt, entry->dma_dir);
>> + dma_buf_detach(dmabuf, entry->a);
>> + dma_buf_put(dmabuf);
>> +
>> + list_del(&entry->list);
>> + kfree(entry);
>> +}
>> +
>> +static void tegra_vde_delayed_unmap(struct work_struct *work)
>> +{
>> + struct tegra_vde_cache_entry *entry;
>> +
>> + entry = container_of(work, struct tegra_vde_cache_entry,
>> + dwork.work);
>> +
>> + mutex_lock(&entry->vde->map_lock);
>> + tegra_vde_release_entry(entry);
>> + mutex_unlock(&entry->vde->map_lock);
>
> From smatch:
>
> drivers/staging/media/tegra-vde/dmabuf-cache.c:55 tegra_vde_delayed_unmap() error: dereferencing freed memory 'entry'
That's a very good catch, thanks you very much! I'm keep forgetting about smatch, it's
a useful tool. And unfortunately I can't KASAN the driver because ARM32 doesn't
support KASAN in upstream and Xorg hangs with the unofficial patch that adds support
for the KASAN.
[snip]
>> + entry->dma_dir = dma_dir;
>> + entry->iova = iova;
>
> From smatch:
>
> drivers/staging/media/tegra-vde/dmabuf-cache.c:133 tegra_vde_dmabuf_cache_map() error: uninitialized symbol 'iova'.
This is fine, but indeed won't hurt to explicitly initialize to NULL.
Thanks again!
