Quoting Christian König (2017-09-04 14:27:33)
> From: Christian König <christian.koenig(a)amd.com>
>
> The logic is buggy and unnecessary complex. When dma_fence_get_rcu() fails to
> acquire a reference it doesn't necessary mean that there is no fence at all.
>
> It usually mean that the fence was replaced by a new one and in this situation
> we certainly want to have the new one as result and *NOT* NULL.
Which is not guaranteed by the code you wrote either.
The point of the comment is that the mb is only inside the successful
kref_atomic_inc_unless_zero, and that only after that mb do you know
whether or not you have the current fence.
You can argue that you want to replace the
if (!dma_fence_get_rcu())
return NULL
with
if (!dma_fence_get_rcu()
continue;
but it would be incorrect to say that by simply ignoring the
post-condition check that you do have the right fence.
-Chris
From: Christian König <christian.koenig(a)amd.com>
When dma_fence_get_rcu() fails to acquire a reference it doesn't necessary
mean that there is no fence at all.
It usually mean that the fence was replaced by a new one and in this situation
we certainly want to have the new one as result and *NOT* NULL.
v2: Keep extra check after dma_fence_get_rcu().
Signed-off-by: Christian König <christian.koenig(a)amd.com>
Cc: Chris Wilson <chris(a)chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch>
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: linux-media(a)vger.kernel.org
Cc: dri-devel(a)lists.freedesktop.org
Cc: linaro-mm-sig(a)lists.linaro.org
---
include/linux/dma-fence.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 0a186c4..f4f23cb 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -248,9 +248,12 @@ dma_fence_get_rcu_safe(struct dma_fence * __rcu *fencep)
struct dma_fence *fence;
fence = rcu_dereference(*fencep);
- if (!fence || !dma_fence_get_rcu(fence))
+ if (!fence)
return NULL;
+ if (!dma_fence_get_rcu(fence))
+ continue;
+
/* The atomic_inc_not_zero() inside dma_fence_get_rcu()
* provides a full memory barrier upon success (such as now).
* This is paired with the write barrier from assigning
--
2.7.4
From: Colin Ian King <colin.king(a)canonical.com>
sg_table is being initialized and is never read before it is updated
again later on, hence making the initialization redundant. Remove
the initialization.
Detected by clang scan-build:
"warning: Value stored to 'sg_table' during its initialization is
never read"
Signed-off-by: Colin Ian King <colin.king(a)canonical.com>
---
drivers/dma-buf/dma-buf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 4a038dcf5361..bc1cb284111c 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -625,7 +625,7 @@ EXPORT_SYMBOL_GPL(dma_buf_detach);
struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
enum dma_data_direction direction)
{
- struct sg_table *sg_table = ERR_PTR(-EINVAL);
+ struct sg_table *sg_table;
might_sleep();
--
2.14.1
