Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

8 participants
2915 discussions

Re: [Linaro-mm-sig] [PATCH v3 0/4] dma-buf: Flag vmap'ed memory as system or I/O memory

by Sam Ravnborg

Hi Thomas. > > > > struct simap { > > union { > > void __iomem *vaddr_iomem; > > void *vaddr; > > }; > > bool is_iomem; > > }; > > > > Where simap is a shorthand for system_iomem_map > > And it could al be stuffed into a include/linux/simap.h file. > > > > Not totally sold on the simap name - but wanted to come up with > > something. > > Yes. Others, myself included, have suggested to use a name that does not > imply a connection to the dma-buf framework, but no one has come up with > a good name. > > I strongly dislike simap, as it's entirely non-obvious what it does. > dma-buf-map is not actually wrong. The structures represents the mapping > of a dma-able buffer in most cases. > > > > > With this approach users do not have to pull in dma-buf to use it and > > users will not confuse that this is only for dma-buf usage. > > There's no need to enable dma-buf. It's all in the header file without > dependencies on dma-buf. It's really just the name. > > But there's something else to take into account. The whole issue here is > that the buffer is disconnected from its originating driver, so we don't > know which kind of memory ops we have to use. Thinking about it, I > realized that no one else seemed to have this problem until now. > Otherwise there would be a solution already. So maybe the dma-buf > framework *is* the native use case for this data structure. We have at least: linux/fb.h: union { char __iomem *screen_base; /* Virtual address */ char *screen_buffer; }; Which solve more or less the same problem. > Anyway, if a better name than dma-buf-map comes in, I'm willing to > rename the thing. Otherwise I intend to merge the patchset by the end of > the week. Well, the main thing is that I think this shoud be moved away from dma-buf. But if indeed dma-buf is the only relevant user in drm then I am totally fine with the current naming. One alternative named that popped up in my head: struct sys_io_map {} But again, if this is kept in dma-buf then I am fine with the current naming. In other words, if you continue to think this is mostly a dma-buf thing all three patches are: Acked-by: Sam Ravnborg <sam(a)ravnborg.org> Sam

4 years, 9 months

Re: [Linaro-mm-sig] [PATCH] drm/vc4: Deleted the drm_device declaration

by Daniel Vetter

On Fri, Sep 25, 2020 at 04:51:38PM +0800, Tian Tao wrote: > drm_modeset_lock.h already declares struct drm_device, so there's no > need to declare it in vc4_drv.h > > Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> Just an aside, when submitting patches please use scripts/get_maintainers.pl to generate the recipient list. Looking through past few patches from you it seems fairly arbitrary and often misses the actual maintainers for a given piece of code, which increases the odds the patch will get lost a lot. E.g. for this one I'm only like the 5th or so fallback person, and the main maintainer isn't on the recipient list. Cheeers, Daniel > --- > drivers/gpu/drm/vc4/vc4_drv.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h > index 8c8d96b..8717a1c 100644 > --- a/drivers/gpu/drm/vc4/vc4_drv.h > +++ b/drivers/gpu/drm/vc4/vc4_drv.h > @@ -19,7 +19,6 @@ > > #include "uapi/drm/vc4_drm.h" > > -struct drm_device; > struct drm_gem_object; > > /* Don't forget to update vc4_bo.c: bo_type_names[] when adding to > -- > 2.7.4 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

4 years, 9 months

Re: [Linaro-mm-sig] [PATCH -next] dma-buf: heaps: Remove unused variable ret

by Christian König

Am 22.09.20 um 09:53 schrieb Zou Wei: > This patch fixes below warnings reported by coccicheck > > ./drivers/dma-buf/heaps/heap-helpers.c:202:5-8: Unneeded variable: "ret". Return "0" on line 215 > > Signed-off-by: Zou Wei <zou_wei(a)huawei.com> Acked-by: Christian König <christian.koenig(a)amd.com> Going to pick this up for drm-misc-next. > --- > drivers/dma-buf/heaps/heap-helpers.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/heaps/heap-helpers.c b/drivers/dma-buf/heaps/heap-helpers.c > index d0696cf..7969510 100644 > --- a/drivers/dma-buf/heaps/heap-helpers.c > +++ b/drivers/dma-buf/heaps/heap-helpers.c > @@ -199,7 +199,6 @@ static int dma_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, > { > struct heap_helper_buffer *buffer = dmabuf->priv; > struct dma_heaps_attachment *a; > - int ret = 0; > > mutex_lock(&buffer->lock); > > @@ -212,7 +211,7 @@ static int dma_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, > } > mutex_unlock(&buffer->lock); > > - return ret; > + return 0; > } > > static int dma_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf,

4 years, 9 months

[PATCH] dmabuf: fix NULL pointer dereference in dma_buf_release()

by Charan Teja Reddy

NULL pointer dereference is observed while exporting the dmabuf but failed to allocate the 'struct file' which results into the dropping of the allocated dentry corresponding to this file in the dmabuf fs, which is ending up in dma_buf_release() and accessing the uninitialzed dentry->d_fsdata. Call stack on 5.4 is below: dma_buf_release+0x2c/0x254 drivers/dma-buf/dma-buf.c:88 __dentry_kill+0x294/0x31c fs/dcache.c:584 dentry_kill fs/dcache.c:673 [inline] dput+0x250/0x380 fs/dcache.c:859 path_put+0x24/0x40 fs/namei.c:485 alloc_file_pseudo+0x1a4/0x200 fs/file_table.c:235 dma_buf_getfile drivers/dma-buf/dma-buf.c:473 [inline] dma_buf_export+0x25c/0x3ec drivers/dma-buf/dma-buf.c:585 Fix this by checking for the valid pointer in the dentry->d_fsdata. Fixes: 4ab59c3c638c ("dma-buf: Move dma_buf_release() from fops to dentry_ops") Cc: <stable(a)vger.kernel.org> [5.7+] Signed-off-by: Charan Teja Reddy <charante(a)codeaurora.org> --- drivers/dma-buf/dma-buf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 58564d82..844967f 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -59,6 +59,8 @@ static void dma_buf_release(struct dentry *dentry) struct dma_buf *dmabuf; dmabuf = dentry->d_fsdata; + if (unlikely(!dmabuf)) + return; BUG_ON(dmabuf->vmapping_counter); -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation

4 years, 9 months

[PATCH AUTOSEL 4.19 003/206] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)

by Sasha Levin

From: Chris Wilson <chris(a)chris-wilson.co.uk> [ Upstream commit 9c98f021e4e717ffd9948fa65340ea3ef12b7935 ] Make dma_fence_enable_sw_signaling() behave like its dma_fence_add_callback() and dma_fence_default_wait() counterparts and perform the test to enable signaling under the fence->lock, along with the action to do so. This ensure that should an implementation be trying to flush the cb_list (by signaling) on retirement before freeing the fence, it can do so in a race-free manner. See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked with dma_fence_signal"). v2: Refactor all 3 enable_signaling paths to use a common function. v3: Don't argue, just keep the tracepoint in the existing spot. Signed-off-by: Chris Wilson <chris(a)chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin(a)intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin(a)intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191004101140.32713-1-chris@… Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/dma-buf/dma-fence.c | 78 +++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 43 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 1551ca7df3941..8586cc05def17 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -244,6 +244,30 @@ void dma_fence_free(struct dma_fence *fence) } EXPORT_SYMBOL(dma_fence_free); +static bool __dma_fence_enable_signaling(struct dma_fence *fence) +{ + bool was_set; + + lockdep_assert_held(fence->lock); + + was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, + &fence->flags); + + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) + return false; + + if (!was_set && fence->ops->enable_signaling) { + trace_dma_fence_enable_signal(fence); + + if (!fence->ops->enable_signaling(fence)) { + dma_fence_signal_locked(fence); + return false; + } + } + + return true; +} + /** * dma_fence_enable_sw_signaling - enable signaling on fence * @fence: the fence to enable @@ -256,19 +280,12 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence) { unsigned long flags; - if (!test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, - &fence->flags) && - !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) && - fence->ops->enable_signaling) { - trace_dma_fence_enable_signal(fence); - - spin_lock_irqsave(fence->lock, flags); - - if (!fence->ops->enable_signaling(fence)) - dma_fence_signal_locked(fence); + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) + return; - spin_unlock_irqrestore(fence->lock, flags); - } + spin_lock_irqsave(fence->lock, flags); + __dma_fence_enable_signaling(fence); + spin_unlock_irqrestore(fence->lock, flags); } EXPORT_SYMBOL(dma_fence_enable_sw_signaling); @@ -302,7 +319,6 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb, { unsigned long flags; int ret = 0; - bool was_set; if (WARN_ON(!fence || !func)) return -EINVAL; @@ -314,25 +330,14 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb, spin_lock_irqsave(fence->lock, flags); - was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, - &fence->flags); - - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) - ret = -ENOENT; - else if (!was_set && fence->ops->enable_signaling) { - trace_dma_fence_enable_signal(fence); - - if (!fence->ops->enable_signaling(fence)) { - dma_fence_signal_locked(fence); - ret = -ENOENT; - } - } - - if (!ret) { + if (__dma_fence_enable_signaling(fence)) { cb->func = func; list_add_tail(&cb->node, &fence->cb_list); - } else + } else { INIT_LIST_HEAD(&cb->node); + ret = -ENOENT; + } + spin_unlock_irqrestore(fence->lock, flags); return ret; @@ -432,7 +437,6 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout) struct default_wait_cb cb; unsigned long flags; signed long ret = timeout ? timeout : 1; - bool was_set; if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) return ret; @@ -444,21 +448,9 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout) goto out; } - was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, - &fence->flags); - - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) + if (!__dma_fence_enable_signaling(fence)) goto out; - if (!was_set && fence->ops->enable_signaling) { - trace_dma_fence_enable_signal(fence); - - if (!fence->ops->enable_signaling(fence)) { - dma_fence_signal_locked(fence); - goto out; - } - } - if (!timeout) { ret = 0; goto out; -- 2.25.1

4 years, 9 months

[PATCH AUTOSEL 5.4 009/330] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)

by Sasha Levin

From: Chris Wilson <chris(a)chris-wilson.co.uk> [ Upstream commit 9c98f021e4e717ffd9948fa65340ea3ef12b7935 ] Make dma_fence_enable_sw_signaling() behave like its dma_fence_add_callback() and dma_fence_default_wait() counterparts and perform the test to enable signaling under the fence->lock, along with the action to do so. This ensure that should an implementation be trying to flush the cb_list (by signaling) on retirement before freeing the fence, it can do so in a race-free manner. See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked with dma_fence_signal"). v2: Refactor all 3 enable_signaling paths to use a common function. v3: Don't argue, just keep the tracepoint in the existing spot. Signed-off-by: Chris Wilson <chris(a)chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin(a)intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin(a)intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20191004101140.32713-1-chris@… Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/dma-buf/dma-fence.c | 78 +++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 43 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 2c136aee3e794..052a41e2451c1 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -273,6 +273,30 @@ void dma_fence_free(struct dma_fence *fence) } EXPORT_SYMBOL(dma_fence_free); +static bool __dma_fence_enable_signaling(struct dma_fence *fence) +{ + bool was_set; + + lockdep_assert_held(fence->lock); + + was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, + &fence->flags); + + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) + return false; + + if (!was_set && fence->ops->enable_signaling) { + trace_dma_fence_enable_signal(fence); + + if (!fence->ops->enable_signaling(fence)) { + dma_fence_signal_locked(fence); + return false; + } + } + + return true; +} + /** * dma_fence_enable_sw_signaling - enable signaling on fence * @fence: the fence to enable @@ -285,19 +309,12 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence) { unsigned long flags; - if (!test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, - &fence->flags) && - !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) && - fence->ops->enable_signaling) { - trace_dma_fence_enable_signal(fence); - - spin_lock_irqsave(fence->lock, flags); - - if (!fence->ops->enable_signaling(fence)) - dma_fence_signal_locked(fence); + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) + return; - spin_unlock_irqrestore(fence->lock, flags); - } + spin_lock_irqsave(fence->lock, flags); + __dma_fence_enable_signaling(fence); + spin_unlock_irqrestore(fence->lock, flags); } EXPORT_SYMBOL(dma_fence_enable_sw_signaling); @@ -331,7 +348,6 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb, { unsigned long flags; int ret = 0; - bool was_set; if (WARN_ON(!fence || !func)) return -EINVAL; @@ -343,25 +359,14 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb, spin_lock_irqsave(fence->lock, flags); - was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, - &fence->flags); - - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) - ret = -ENOENT; - else if (!was_set && fence->ops->enable_signaling) { - trace_dma_fence_enable_signal(fence); - - if (!fence->ops->enable_signaling(fence)) { - dma_fence_signal_locked(fence); - ret = -ENOENT; - } - } - - if (!ret) { + if (__dma_fence_enable_signaling(fence)) { cb->func = func; list_add_tail(&cb->node, &fence->cb_list); - } else + } else { INIT_LIST_HEAD(&cb->node); + ret = -ENOENT; + } + spin_unlock_irqrestore(fence->lock, flags); return ret; @@ -461,7 +466,6 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout) struct default_wait_cb cb; unsigned long flags; signed long ret = timeout ? timeout : 1; - bool was_set; if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) return ret; @@ -473,21 +477,9 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout) goto out; } - was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, - &fence->flags); - - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) + if (!__dma_fence_enable_signaling(fence)) goto out; - if (!was_set && fence->ops->enable_signaling) { - trace_dma_fence_enable_signal(fence); - - if (!fence->ops->enable_signaling(fence)) { - dma_fence_signal_locked(fence); - goto out; - } - } - if (!timeout) { ret = 0; goto out; -- 2.25.1

4 years, 9 months

Changing vma->vm_file in dma_buf_mmap()

by Christian König

Hi Andrew, I'm the new DMA-buf maintainer and Daniel and others came up with patches extending the use of the dma_buf_mmap() function. Now this function is doing something a bit odd by changing the vma->vm_file while installing a VMA in the mmap() system call The background here is that DMA-buf allows device drivers to export buffer which are then imported into another device driver. The mmap() handler of the importing device driver then find that the pgoff belongs to the exporting device and so redirects the mmap() call there. In other words user space calls mmap() on one file descriptor, but get a different one mapped into your virtual address space. My question is now: Is that legal or can you think of something which breaks here? If it's not legal we should probably block any new users of the dma_buf_mmap() function and consider what should happen with the two existing ones. If that is legal I would like to document this by adding a new vma_set_file() function which does the necessary reference count dance. Thanks in advance, Christian.

4 years, 9 months

[PATCH] dma-resv: lockdep-prime address_space->i_mmap_rwsem for dma-resv

by Daniel Vetter

GPU drivers need this in their shrinkers, to be able to throw out mmap'ed buffers. Note that we also need dma_resv_lock in shrinkers, but that loop is resolved by trylocking in shrinkers. So full hierarchy is now (ignore some of the other branches we already have primed): mmap_read_lock -> dma_resv -> shrinkers -> i_mmap_lock_write I hope that's not inconsistent with anything mm or fs does, adding relevant people. Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: Dave Chinner <david(a)fromorbit.com> Cc: Qian Cai <cai(a)lca.pw> Cc: linux-xfs(a)vger.kernel.org Cc: linux-fsdevel(a)vger.kernel.org Cc: Thomas Hellström (Intel) <thomas_os(a)shipmail.org> Cc: Andrew Morton <akpm(a)linux-foundation.org> Cc: Jason Gunthorpe <jgg(a)mellanox.com> Cc: linux-mm(a)kvack.org Cc: linux-rdma(a)vger.kernel.org Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> --- drivers/dma-buf/dma-resv.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 0e6675ec1d11..9678162a4ac5 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -104,12 +104,14 @@ static int __init dma_resv_lockdep(void) struct mm_struct *mm = mm_alloc(); struct ww_acquire_ctx ctx; struct dma_resv obj; + struct address_space mapping; int ret; if (!mm) return -ENOMEM; dma_resv_init(&obj); + address_space_init_once(&mapping); mmap_read_lock(mm); ww_acquire_init(&ctx, &reservation_ww_class); @@ -117,6 +119,9 @@ static int __init dma_resv_lockdep(void) if (ret == -EDEADLK) dma_resv_lock_slow(&obj, &ctx); fs_reclaim_acquire(GFP_KERNEL); + /* for unmap_mapping_range on trylocked buffer objects in shrinkers */ + i_mmap_lock_write(&mapping); + i_mmap_unlock_write(&mapping); #ifdef CONFIG_MMU_NOTIFIER lock_map_acquire(&__mmu_notifier_invalidate_range_start_map); __dma_fence_might_wait(); -- 2.27.0

4 years, 9 months

Re: [Linaro-mm-sig] [PATCH] drm/scheduler: fix sched_fence.c kernel-doc warnings

by Christian König

Am 09.09.20 um 09:57 schrieb Tian Tao: > Fix kernel-doc warnings. > drivers/gpu/drm/scheduler/sched_fence.c:110: warning: Function parameter or > member 'f' not described in 'drm_sched_fence_release_scheduled' > drivers/gpu/drm/scheduler/sched_fence.c:110: warning: Excess function > parameter 'fence' description in 'drm_sched_fence_release_scheduled' > > Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> Reviewed-by: Christian König <christian.koenig(a)amd.com> > --- > drivers/gpu/drm/scheduler/sched_fence.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/scheduler/sched_fence.c b/drivers/gpu/drm/scheduler/sched_fence.c > index 8b45c3a1b84..69de2c7 100644 > --- a/drivers/gpu/drm/scheduler/sched_fence.c > +++ b/drivers/gpu/drm/scheduler/sched_fence.c > @@ -101,7 +101,7 @@ static void drm_sched_fence_free(struct rcu_head *rcu) > /** > * drm_sched_fence_release_scheduled - callback that fence can be freed > * > - * @fence: fence > + * @f: fence > * > * This function is called when the reference count becomes zero. > * It just RCU schedules freeing up the fence.

4 years, 9 months

Re: [Linaro-mm-sig] [PATCH v10 30/30] videobuf2: use sgtable-based scatterlist wrappers

by Marek Szyprowski

Hi Tomasz, On 07.09.2020 15:07, Tomasz Figa wrote: > On Fri, Sep 4, 2020 at 3:35 PM Marek Szyprowski > <m.szyprowski(a)samsung.com> wrote: >> Use recently introduced common wrappers operating directly on the struct >> sg_table objects and scatterlist page iterators to make the code a bit >> more compact, robust, easier to follow and copy/paste safe. >> >> No functional change, because the code already properly did all the >> scatterlist related calls. >> >> Signed-off-by: Marek Szyprowski <m.szyprowski(a)samsung.com> >> Reviewed-by: Robin Murphy <robin.murphy(a)arm.com> >> --- >> .../common/videobuf2/videobuf2-dma-contig.c | 34 ++++++++----------- >> .../media/common/videobuf2/videobuf2-dma-sg.c | 32 +++++++---------- >> .../common/videobuf2/videobuf2-vmalloc.c | 12 +++---- >> 3 files changed, 31 insertions(+), 47 deletions(-) >> > Thanks for the patch! Please see my comments inline. > >> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c >> index ec3446cc45b8..1b242d844dde 100644 >> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c >> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c >> @@ -58,10 +58,10 @@ static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt) >> unsigned int i; >> unsigned long size = 0; >> >> - for_each_sg(sgt->sgl, s, sgt->nents, i) { >> + for_each_sgtable_dma_sg(sgt, s, i) { >> if (sg_dma_address(s) != expected) >> break; >> - expected = sg_dma_address(s) + sg_dma_len(s); >> + expected += sg_dma_len(s); >> size += sg_dma_len(s); >> } >> return size; >> @@ -103,8 +103,7 @@ static void vb2_dc_prepare(void *buf_priv) >> if (!sgt) >> return; >> >> - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents, >> - buf->dma_dir); >> + dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir); >> } >> >> static void vb2_dc_finish(void *buf_priv) >> @@ -115,7 +114,7 @@ static void vb2_dc_finish(void *buf_priv) >> if (!sgt) >> return; >> >> - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir); >> + dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir); >> } >> >> /*********************************************/ >> @@ -275,8 +274,8 @@ static void vb2_dc_dmabuf_ops_detach(struct dma_buf *dbuf, >> * memory locations do not require any explicit cache >> * maintenance prior or after being used by the device. >> */ >> - dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents, >> - attach->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); >> + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir, >> + DMA_ATTR_SKIP_CPU_SYNC); >> sg_free_table(sgt); >> kfree(attach); >> db_attach->priv = NULL; >> @@ -301,8 +300,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map( >> >> /* release any previous cache */ >> if (attach->dma_dir != DMA_NONE) { >> - dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents, >> - attach->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); >> + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir, >> + DMA_ATTR_SKIP_CPU_SYNC); >> attach->dma_dir = DMA_NONE; >> } >> >> @@ -310,9 +309,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map( >> * mapping to the client with new direction, no cache sync >> * required see comment in vb2_dc_dmabuf_ops_detach() >> */ >> - sgt->nents = dma_map_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents, >> - dma_dir, DMA_ATTR_SKIP_CPU_SYNC); >> - if (!sgt->nents) { >> + if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, >> + DMA_ATTR_SKIP_CPU_SYNC)) { >> pr_err("failed to map scatterlist\n"); >> mutex_unlock(lock); >> return ERR_PTR(-EIO); > As opposed to dma_map_sg_attrs(), dma_map_sgtable() now returns an > error code on its own. Is it expected to ignore it and return -EIO? Those errors are more or less propagated to userspace and -EIO has been already widely documented in V4L2 documentation as the error code for the most of the V4L2 ioctls. I don't want to change it. A possible -EINVAL returned from dma_map_sgtable() was just one of the 'generic' error codes, not very descriptive in that case. Probably the main problem here is that dma_map_sg() and friend doesn't return any error codes... > ... Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland

4 years, 10 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig