On Mon, Oct 21, 2019 at 01:15:22PM +0200, Christian König wrote:
The attachment list is now protected by the dma_resv object. So we can drop holding this lock to allow concurrent attach and detach operations.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/dma-buf/dma-buf.c | 16 ---------------- 1 file changed, 16 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 753be84b5fd6..c736e67ae1a1 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -685,8 +685,6 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, attach->dmabuf = dmabuf; attach->dynamic_mapping = dynamic_mapping;
- mutex_lock(&dmabuf->lock);
- if (dmabuf->ops->attach) { ret = dmabuf->ops->attach(dmabuf, attach); if (ret)
@@ -696,8 +694,6 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, list_add(&attach->node, &dmabuf->attachments); dma_resv_unlock(dmabuf->resv);
- mutex_unlock(&dmabuf->lock);
This changes the rules, now ->attach/->detach and the list manipulation aren't done under the same lock anymore. I don't think this matters, but imo good to mention in the commit message.
- /* When either the importer or the exporter can't handle dynamic
- mappings we cache the mapping here to avoid issues with the
- reservation object lock.
@@ -726,7 +722,6 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev, err_attach: kfree(attach);
- mutex_unlock(&dmabuf->lock); return ERR_PTR(ret);
err_unlock: @@ -776,14 +771,12 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) dma_resv_unlock(attach->dmabuf->resv); }
- mutex_lock(&dmabuf->lock); dma_resv_lock(dmabuf->resv, NULL); list_del(&attach->node); dma_resv_unlock(dmabuf->resv); if (dmabuf->ops->detach) dmabuf->ops->detach(dmabuf, attach);
- mutex_unlock(&dmabuf->lock); kfree(attach);
} EXPORT_SYMBOL_GPL(dma_buf_detach); @@ -1247,14 +1240,6 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused) "size", "flags", "mode", "count", "ino"); list_for_each_entry(buf_obj, &db_list.head, list_node) {
ret = mutex_lock_interruptible(&buf_obj->lock);
if (ret) {
seq_puts(s,
"\tERROR locking buffer object: skipping\n");
continue;
}
This will mildly conflict with the revised version of patch 1 (since the dma_resv_lock needs to be here).
With both nits addressed:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
seq_printf(s, "%08zu\t%08x\t%08x\t%08ld\t%s\t%08lu\t%s\n", buf_obj->size, buf_obj->file->f_flags, buf_obj->file->f_mode,
@@ -1307,7 +1292,6 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused) count++; size += buf_obj->size;
}mutex_unlock(&buf_obj->lock);
seq_printf(s, "\nTotal %d objects, %zu bytes\n", count, size); -- 2.17.1