All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
@ 2020-12-11 15:58 ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-11 15:58 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Thomas Zimmermann, Sumit Semwal, Christian König,
	linux-media, linaro-mm-sig

Also try to clarify a bit when dma_buf_begin/end_cpu_access should
be called.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
 include/linux/dma-buf.h   | 25 +++++++++----------------
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index e63684d4cd90..a12fdffa130f 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
  *   vmalloc space might be limited and result in vmap calls failing.
  *
  *   Interfaces::
+ *
  *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
  *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
  *
  *   The vmap call can fail if there is no vmap support in the exporter, or if
- *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
- *   that the dma-buf layer keeps a reference count for all vmap access and
- *   calls down into the exporter's vmap function only when no vmapping exists,
- *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
- *   provided by taking the dma_buf->lock mutex.
+ *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
+ *   count for all vmap access and calls down into the exporter's vmap function
+ *   only when no vmapping exists, and only unmaps it once. Protection against
+ *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
  *
  * - For full compatibility on the importer side with existing userspace
  *   interfaces, which might already support mmap'ing buffers. This is needed in
@@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
  * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
  * it guaranteed to be coherent with other DMA access.
  *
+ * This function will also wait for any DMA transactions tracked through
+ * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
+ * synchronization this function will only ensure cache coherency, callers must
+ * ensure synchronization with such DMA transactions on their own.
+ *
  * Can return negative error values, returns 0 on success.
  */
 int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
@@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
  * This call may fail due to lack of virtual mapping address space.
  * These calls are optional in drivers. The intended use for them
  * is for mapping objects linear in kernel space for high use objects.
- * Please attempt to use kmap/kunmap before thinking about these interfaces.
+ *
+ * To ensure coherency users must call dma_buf_begin_cpu_access() and
+ * dma_buf_end_cpu_access() around any cpu access performed through this
+ * mapping.
  *
  * Returns 0 on success, or a negative errno code otherwise.
  */
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index cf72699cb2bc..7eca37c8b10c 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -183,24 +183,19 @@ struct dma_buf_ops {
 	 * @begin_cpu_access:
 	 *
 	 * This is called from dma_buf_begin_cpu_access() and allows the
-	 * exporter to ensure that the memory is actually available for cpu
-	 * access - the exporter might need to allocate or swap-in and pin the
-	 * backing storage. The exporter also needs to ensure that cpu access is
-	 * coherent for the access direction. The direction can be used by the
-	 * exporter to optimize the cache flushing, i.e. access with a different
+	 * exporter to ensure that the memory is actually coherent for cpu
+	 * access. The exporter also needs to ensure that cpu access is coherent
+	 * for the access direction. The direction can be used by the exporter
+	 * to optimize the cache flushing, i.e. access with a different
 	 * direction (read instead of write) might return stale or even bogus
 	 * data (e.g. when the exporter needs to copy the data to temporary
 	 * storage).
 	 *
-	 * This callback is optional.
+	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
+	 * command for userspace mappings established through @mmap, and also
+	 * for kernel mappings established with @vmap.
 	 *
-	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
-	 * from userspace (where storage shouldn't be pinned to avoid handing
-	 * de-factor mlock rights to userspace) and for the kernel-internal
-	 * users of the various kmap interfaces, where the backing storage must
-	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
-	 * there's no in-kernel users of the kmap interfaces yet this isn't a
-	 * real problem.
+	 * This callback is optional.
 	 *
 	 * Returns:
 	 *
@@ -216,9 +211,7 @@ struct dma_buf_ops {
 	 *
 	 * This is called from dma_buf_end_cpu_access() when the importer is
 	 * done accessing the CPU. The exporter can use this to flush caches and
-	 * unpin any resources pinned in @begin_cpu_access.
-	 * The result of any dma_buf kmap calls after end_cpu_access is
-	 * undefined.
+	 * undo anything else done in @begin_cpu_access.
 	 *
 	 * This callback is optional.
 	 *
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
@ 2020-12-11 15:58 ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-11 15:58 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, linaro-mm-sig,
	Thomas Zimmermann, Daniel Vetter, Christian König,
	linux-media

Also try to clarify a bit when dma_buf_begin/end_cpu_access should
be called.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
 include/linux/dma-buf.h   | 25 +++++++++----------------
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index e63684d4cd90..a12fdffa130f 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
  *   vmalloc space might be limited and result in vmap calls failing.
  *
  *   Interfaces::
+ *
  *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
  *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
  *
  *   The vmap call can fail if there is no vmap support in the exporter, or if
- *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
- *   that the dma-buf layer keeps a reference count for all vmap access and
- *   calls down into the exporter's vmap function only when no vmapping exists,
- *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
- *   provided by taking the dma_buf->lock mutex.
+ *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
+ *   count for all vmap access and calls down into the exporter's vmap function
+ *   only when no vmapping exists, and only unmaps it once. Protection against
+ *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
  *
  * - For full compatibility on the importer side with existing userspace
  *   interfaces, which might already support mmap'ing buffers. This is needed in
@@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
  * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
  * it guaranteed to be coherent with other DMA access.
  *
+ * This function will also wait for any DMA transactions tracked through
+ * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
+ * synchronization this function will only ensure cache coherency, callers must
+ * ensure synchronization with such DMA transactions on their own.
+ *
  * Can return negative error values, returns 0 on success.
  */
 int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
@@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
  * This call may fail due to lack of virtual mapping address space.
  * These calls are optional in drivers. The intended use for them
  * is for mapping objects linear in kernel space for high use objects.
- * Please attempt to use kmap/kunmap before thinking about these interfaces.
+ *
+ * To ensure coherency users must call dma_buf_begin_cpu_access() and
+ * dma_buf_end_cpu_access() around any cpu access performed through this
+ * mapping.
  *
  * Returns 0 on success, or a negative errno code otherwise.
  */
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index cf72699cb2bc..7eca37c8b10c 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -183,24 +183,19 @@ struct dma_buf_ops {
 	 * @begin_cpu_access:
 	 *
 	 * This is called from dma_buf_begin_cpu_access() and allows the
-	 * exporter to ensure that the memory is actually available for cpu
-	 * access - the exporter might need to allocate or swap-in and pin the
-	 * backing storage. The exporter also needs to ensure that cpu access is
-	 * coherent for the access direction. The direction can be used by the
-	 * exporter to optimize the cache flushing, i.e. access with a different
+	 * exporter to ensure that the memory is actually coherent for cpu
+	 * access. The exporter also needs to ensure that cpu access is coherent
+	 * for the access direction. The direction can be used by the exporter
+	 * to optimize the cache flushing, i.e. access with a different
 	 * direction (read instead of write) might return stale or even bogus
 	 * data (e.g. when the exporter needs to copy the data to temporary
 	 * storage).
 	 *
-	 * This callback is optional.
+	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
+	 * command for userspace mappings established through @mmap, and also
+	 * for kernel mappings established with @vmap.
 	 *
-	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
-	 * from userspace (where storage shouldn't be pinned to avoid handing
-	 * de-factor mlock rights to userspace) and for the kernel-internal
-	 * users of the various kmap interfaces, where the backing storage must
-	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
-	 * there's no in-kernel users of the kmap interfaces yet this isn't a
-	 * real problem.
+	 * This callback is optional.
 	 *
 	 * Returns:
 	 *
@@ -216,9 +211,7 @@ struct dma_buf_ops {
 	 *
 	 * This is called from dma_buf_end_cpu_access() when the importer is
 	 * done accessing the CPU. The exporter can use this to flush caches and
-	 * unpin any resources pinned in @begin_cpu_access.
-	 * The result of any dma_buf kmap calls after end_cpu_access is
-	 * undefined.
+	 * undo anything else done in @begin_cpu_access.
 	 *
 	 * This callback is optional.
 	 *
-- 
2.29.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [Intel-gfx] [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
@ 2020-12-11 15:58 ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-11 15:58 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Sumit Semwal,
	linaro-mm-sig, Thomas Zimmermann, Daniel Vetter,
	Christian König, linux-media

Also try to clarify a bit when dma_buf_begin/end_cpu_access should
be called.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
 include/linux/dma-buf.h   | 25 +++++++++----------------
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index e63684d4cd90..a12fdffa130f 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
  *   vmalloc space might be limited and result in vmap calls failing.
  *
  *   Interfaces::
+ *
  *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
  *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
  *
  *   The vmap call can fail if there is no vmap support in the exporter, or if
- *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
- *   that the dma-buf layer keeps a reference count for all vmap access and
- *   calls down into the exporter's vmap function only when no vmapping exists,
- *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
- *   provided by taking the dma_buf->lock mutex.
+ *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
+ *   count for all vmap access and calls down into the exporter's vmap function
+ *   only when no vmapping exists, and only unmaps it once. Protection against
+ *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
  *
  * - For full compatibility on the importer side with existing userspace
  *   interfaces, which might already support mmap'ing buffers. This is needed in
@@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
  * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
  * it guaranteed to be coherent with other DMA access.
  *
+ * This function will also wait for any DMA transactions tracked through
+ * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
+ * synchronization this function will only ensure cache coherency, callers must
+ * ensure synchronization with such DMA transactions on their own.
+ *
  * Can return negative error values, returns 0 on success.
  */
 int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
@@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
  * This call may fail due to lack of virtual mapping address space.
  * These calls are optional in drivers. The intended use for them
  * is for mapping objects linear in kernel space for high use objects.
- * Please attempt to use kmap/kunmap before thinking about these interfaces.
+ *
+ * To ensure coherency users must call dma_buf_begin_cpu_access() and
+ * dma_buf_end_cpu_access() around any cpu access performed through this
+ * mapping.
  *
  * Returns 0 on success, or a negative errno code otherwise.
  */
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index cf72699cb2bc..7eca37c8b10c 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -183,24 +183,19 @@ struct dma_buf_ops {
 	 * @begin_cpu_access:
 	 *
 	 * This is called from dma_buf_begin_cpu_access() and allows the
-	 * exporter to ensure that the memory is actually available for cpu
-	 * access - the exporter might need to allocate or swap-in and pin the
-	 * backing storage. The exporter also needs to ensure that cpu access is
-	 * coherent for the access direction. The direction can be used by the
-	 * exporter to optimize the cache flushing, i.e. access with a different
+	 * exporter to ensure that the memory is actually coherent for cpu
+	 * access. The exporter also needs to ensure that cpu access is coherent
+	 * for the access direction. The direction can be used by the exporter
+	 * to optimize the cache flushing, i.e. access with a different
 	 * direction (read instead of write) might return stale or even bogus
 	 * data (e.g. when the exporter needs to copy the data to temporary
 	 * storage).
 	 *
-	 * This callback is optional.
+	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
+	 * command for userspace mappings established through @mmap, and also
+	 * for kernel mappings established with @vmap.
 	 *
-	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
-	 * from userspace (where storage shouldn't be pinned to avoid handing
-	 * de-factor mlock rights to userspace) and for the kernel-internal
-	 * users of the various kmap interfaces, where the backing storage must
-	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
-	 * there's no in-kernel users of the kmap interfaces yet this isn't a
-	 * real problem.
+	 * This callback is optional.
 	 *
 	 * Returns:
 	 *
@@ -216,9 +211,7 @@ struct dma_buf_ops {
 	 *
 	 * This is called from dma_buf_end_cpu_access() when the importer is
 	 * done accessing the CPU. The exporter can use this to flush caches and
-	 * unpin any resources pinned in @begin_cpu_access.
-	 * The result of any dma_buf kmap calls after end_cpu_access is
-	 * undefined.
+	 * undo anything else done in @begin_cpu_access.
 	 *
 	 * This callback is optional.
 	 *
-- 
2.29.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 2/4] dma-buf: some kerneldoc formatting fixes
  2020-12-11 15:58 ` Daniel Vetter
  (?)
@ 2020-12-11 15:58   ` Daniel Vetter
  -1 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-11 15:58 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Thomas Zimmermann, Sumit Semwal, Christian König,
	linux-media, linaro-mm-sig

Noticed while reviewing the output. Adds a bunch more links and fixes
the function interface quoting.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 31 ++++++++++++++++++-------------
 include/linux/dma-buf.h   |  6 +++---
 2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index a12fdffa130f..e1fa6c6f02c4 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -480,7 +480,7 @@ static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
  *
  * 4. Once a driver is done with a shared buffer it needs to call
  *    dma_buf_detach() (after cleaning up any mappings) and then release the
- *    reference acquired with dma_buf_get by calling dma_buf_put().
+ *    reference acquired with dma_buf_get() by calling dma_buf_put().
  *
  * For the detailed semantics exporters are expected to implement see
  * &dma_buf_ops.
@@ -496,9 +496,10 @@ static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
  *			by the exporter. see &struct dma_buf_export_info
  *			for further details.
  *
- * Returns, on success, a newly created dma_buf object, which wraps the
- * supplied private data and operations for dma_buf_ops. On either missing
- * ops, or error in allocating struct dma_buf, will return negative error.
+ * Returns, on success, a newly created struct dma_buf object, which wraps the
+ * supplied private data and operations for struct dma_buf_ops. On either
+ * missing ops, or error in allocating struct dma_buf, will return negative
+ * error.
  *
  * For most cases the easiest way to create @exp_info is through the
  * %DEFINE_DMA_BUF_EXPORT_INFO macro.
@@ -584,7 +585,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
 EXPORT_SYMBOL_GPL(dma_buf_export);
 
 /**
- * dma_buf_fd - returns a file descriptor for the given dma_buf
+ * dma_buf_fd - returns a file descriptor for the given struct dma_buf
  * @dmabuf:	[in]	pointer to dma_buf for which fd is required.
  * @flags:      [in]    flags to give to fd
  *
@@ -608,10 +609,10 @@ int dma_buf_fd(struct dma_buf *dmabuf, int flags)
 EXPORT_SYMBOL_GPL(dma_buf_fd);
 
 /**
- * dma_buf_get - returns the dma_buf structure related to an fd
- * @fd:	[in]	fd associated with the dma_buf to be returned
+ * dma_buf_get - returns the struct dma_buf related to an fd
+ * @fd:	[in]	fd associated with the struct dma_buf to be returned
  *
- * On success, returns the dma_buf structure associated with an fd; uses
+ * On success, returns the struct dma_buf associated with an fd; uses
  * file's refcounting done by fget to increase refcount. returns ERR_PTR
  * otherwise.
  */
@@ -653,8 +654,7 @@ void dma_buf_put(struct dma_buf *dmabuf)
 EXPORT_SYMBOL_GPL(dma_buf_put);
 
 /**
- * dma_buf_dynamic_attach - Add the device to dma_buf's attachments list; optionally,
- * calls attach() of dma_buf_ops to allow device-specific attach functionality
+ * dma_buf_dynamic_attach - Add the device to dma_buf's attachments list
  * @dmabuf:		[in]	buffer to attach device to.
  * @dev:		[in]	device to be attached.
  * @importer_ops:	[in]	importer operations for the attachment
@@ -663,6 +663,9 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
  * Returns struct dma_buf_attachment pointer for this attachment. Attachments
  * must be cleaned up by calling dma_buf_detach().
  *
+ * Optionally this calls &dma_buf_ops.attach to allow device-specific attach
+ * functionality.
+ *
  * Returns:
  *
  * A pointer to newly created &dma_buf_attachment on success, or a negative
@@ -769,12 +772,13 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
 EXPORT_SYMBOL_GPL(dma_buf_attach);
 
 /**
- * dma_buf_detach - Remove the given attachment from dmabuf's attachments list;
- * optionally calls detach() of dma_buf_ops for device-specific detach
+ * dma_buf_detach - Remove the given attachment from dmabuf's attachments list
  * @dmabuf:	[in]	buffer to detach from.
  * @attach:	[in]	attachment to be detached; is free'd after this call.
  *
  * Clean up a device attachment obtained by calling dma_buf_attach().
+ *
+ * Optionally this calls &dma_buf_ops.detach for device-specific detach.
  */
 void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
 {
@@ -1061,11 +1065,12 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
  *   shootdowns would increase the complexity quite a bit.
  *
  *   Interface::
+ *
  *      int dma_buf_mmap(struct dma_buf \*, struct vm_area_struct \*,
  *		       unsigned long);
  *
  *   If the importing subsystem simply provides a special-purpose mmap call to
- *   set up a mapping in userspace, calling do_mmap with dma_buf->file will
+ *   set up a mapping in userspace, calling do_mmap with &dma_buf.file will
  *   equally achieve that for a dma-buf object.
  */
 
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 7eca37c8b10c..43802a31b25d 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -85,10 +85,10 @@ struct dma_buf_ops {
 	/**
 	 * @pin:
 	 *
-	 * This is called by dma_buf_pin and lets the exporter know that the
+	 * This is called by dma_buf_pin() and lets the exporter know that the
 	 * DMA-buf can't be moved any more.
 	 *
-	 * This is called with the dmabuf->resv object locked and is mutual
+	 * This is called with the &dmabuf.resv object locked and is mutual
 	 * exclusive with @cache_sgt_mapping.
 	 *
 	 * This callback is optional and should only be used in limited use
@@ -103,7 +103,7 @@ struct dma_buf_ops {
 	/**
 	 * @unpin:
 	 *
-	 * This is called by dma_buf_unpin and lets the exporter know that the
+	 * This is called by dma_buf_unpin() and lets the exporter know that the
 	 * DMA-buf can be moved again.
 	 *
 	 * This is called with the dmabuf->resv object locked and is mutual
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 2/4] dma-buf: some kerneldoc formatting fixes
@ 2020-12-11 15:58   ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-11 15:58 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, linaro-mm-sig,
	Thomas Zimmermann, Daniel Vetter, Christian König,
	linux-media

Noticed while reviewing the output. Adds a bunch more links and fixes
the function interface quoting.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 31 ++++++++++++++++++-------------
 include/linux/dma-buf.h   |  6 +++---
 2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index a12fdffa130f..e1fa6c6f02c4 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -480,7 +480,7 @@ static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
  *
  * 4. Once a driver is done with a shared buffer it needs to call
  *    dma_buf_detach() (after cleaning up any mappings) and then release the
- *    reference acquired with dma_buf_get by calling dma_buf_put().
+ *    reference acquired with dma_buf_get() by calling dma_buf_put().
  *
  * For the detailed semantics exporters are expected to implement see
  * &dma_buf_ops.
@@ -496,9 +496,10 @@ static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
  *			by the exporter. see &struct dma_buf_export_info
  *			for further details.
  *
- * Returns, on success, a newly created dma_buf object, which wraps the
- * supplied private data and operations for dma_buf_ops. On either missing
- * ops, or error in allocating struct dma_buf, will return negative error.
+ * Returns, on success, a newly created struct dma_buf object, which wraps the
+ * supplied private data and operations for struct dma_buf_ops. On either
+ * missing ops, or error in allocating struct dma_buf, will return negative
+ * error.
  *
  * For most cases the easiest way to create @exp_info is through the
  * %DEFINE_DMA_BUF_EXPORT_INFO macro.
@@ -584,7 +585,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
 EXPORT_SYMBOL_GPL(dma_buf_export);
 
 /**
- * dma_buf_fd - returns a file descriptor for the given dma_buf
+ * dma_buf_fd - returns a file descriptor for the given struct dma_buf
  * @dmabuf:	[in]	pointer to dma_buf for which fd is required.
  * @flags:      [in]    flags to give to fd
  *
@@ -608,10 +609,10 @@ int dma_buf_fd(struct dma_buf *dmabuf, int flags)
 EXPORT_SYMBOL_GPL(dma_buf_fd);
 
 /**
- * dma_buf_get - returns the dma_buf structure related to an fd
- * @fd:	[in]	fd associated with the dma_buf to be returned
+ * dma_buf_get - returns the struct dma_buf related to an fd
+ * @fd:	[in]	fd associated with the struct dma_buf to be returned
  *
- * On success, returns the dma_buf structure associated with an fd; uses
+ * On success, returns the struct dma_buf associated with an fd; uses
  * file's refcounting done by fget to increase refcount. returns ERR_PTR
  * otherwise.
  */
@@ -653,8 +654,7 @@ void dma_buf_put(struct dma_buf *dmabuf)
 EXPORT_SYMBOL_GPL(dma_buf_put);
 
 /**
- * dma_buf_dynamic_attach - Add the device to dma_buf's attachments list; optionally,
- * calls attach() of dma_buf_ops to allow device-specific attach functionality
+ * dma_buf_dynamic_attach - Add the device to dma_buf's attachments list
  * @dmabuf:		[in]	buffer to attach device to.
  * @dev:		[in]	device to be attached.
  * @importer_ops:	[in]	importer operations for the attachment
@@ -663,6 +663,9 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
  * Returns struct dma_buf_attachment pointer for this attachment. Attachments
  * must be cleaned up by calling dma_buf_detach().
  *
+ * Optionally this calls &dma_buf_ops.attach to allow device-specific attach
+ * functionality.
+ *
  * Returns:
  *
  * A pointer to newly created &dma_buf_attachment on success, or a negative
@@ -769,12 +772,13 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
 EXPORT_SYMBOL_GPL(dma_buf_attach);
 
 /**
- * dma_buf_detach - Remove the given attachment from dmabuf's attachments list;
- * optionally calls detach() of dma_buf_ops for device-specific detach
+ * dma_buf_detach - Remove the given attachment from dmabuf's attachments list
  * @dmabuf:	[in]	buffer to detach from.
  * @attach:	[in]	attachment to be detached; is free'd after this call.
  *
  * Clean up a device attachment obtained by calling dma_buf_attach().
+ *
+ * Optionally this calls &dma_buf_ops.detach for device-specific detach.
  */
 void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
 {
@@ -1061,11 +1065,12 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
  *   shootdowns would increase the complexity quite a bit.
  *
  *   Interface::
+ *
  *      int dma_buf_mmap(struct dma_buf \*, struct vm_area_struct \*,
  *		       unsigned long);
  *
  *   If the importing subsystem simply provides a special-purpose mmap call to
- *   set up a mapping in userspace, calling do_mmap with dma_buf->file will
+ *   set up a mapping in userspace, calling do_mmap with &dma_buf.file will
  *   equally achieve that for a dma-buf object.
  */
 
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 7eca37c8b10c..43802a31b25d 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -85,10 +85,10 @@ struct dma_buf_ops {
 	/**
 	 * @pin:
 	 *
-	 * This is called by dma_buf_pin and lets the exporter know that the
+	 * This is called by dma_buf_pin() and lets the exporter know that the
 	 * DMA-buf can't be moved any more.
 	 *
-	 * This is called with the dmabuf->resv object locked and is mutual
+	 * This is called with the &dmabuf.resv object locked and is mutual
 	 * exclusive with @cache_sgt_mapping.
 	 *
 	 * This callback is optional and should only be used in limited use
@@ -103,7 +103,7 @@ struct dma_buf_ops {
 	/**
 	 * @unpin:
 	 *
-	 * This is called by dma_buf_unpin and lets the exporter know that the
+	 * This is called by dma_buf_unpin() and lets the exporter know that the
 	 * DMA-buf can be moved again.
 	 *
 	 * This is called with the dmabuf->resv object locked and is mutual
-- 
2.29.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [Intel-gfx] [PATCH 2/4] dma-buf: some kerneldoc formatting fixes
@ 2020-12-11 15:58   ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-11 15:58 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Sumit Semwal,
	linaro-mm-sig, Thomas Zimmermann, Daniel Vetter,
	Christian König, linux-media

Noticed while reviewing the output. Adds a bunch more links and fixes
the function interface quoting.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 31 ++++++++++++++++++-------------
 include/linux/dma-buf.h   |  6 +++---
 2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index a12fdffa130f..e1fa6c6f02c4 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -480,7 +480,7 @@ static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
  *
  * 4. Once a driver is done with a shared buffer it needs to call
  *    dma_buf_detach() (after cleaning up any mappings) and then release the
- *    reference acquired with dma_buf_get by calling dma_buf_put().
+ *    reference acquired with dma_buf_get() by calling dma_buf_put().
  *
  * For the detailed semantics exporters are expected to implement see
  * &dma_buf_ops.
@@ -496,9 +496,10 @@ static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
  *			by the exporter. see &struct dma_buf_export_info
  *			for further details.
  *
- * Returns, on success, a newly created dma_buf object, which wraps the
- * supplied private data and operations for dma_buf_ops. On either missing
- * ops, or error in allocating struct dma_buf, will return negative error.
+ * Returns, on success, a newly created struct dma_buf object, which wraps the
+ * supplied private data and operations for struct dma_buf_ops. On either
+ * missing ops, or error in allocating struct dma_buf, will return negative
+ * error.
  *
  * For most cases the easiest way to create @exp_info is through the
  * %DEFINE_DMA_BUF_EXPORT_INFO macro.
@@ -584,7 +585,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
 EXPORT_SYMBOL_GPL(dma_buf_export);
 
 /**
- * dma_buf_fd - returns a file descriptor for the given dma_buf
+ * dma_buf_fd - returns a file descriptor for the given struct dma_buf
  * @dmabuf:	[in]	pointer to dma_buf for which fd is required.
  * @flags:      [in]    flags to give to fd
  *
@@ -608,10 +609,10 @@ int dma_buf_fd(struct dma_buf *dmabuf, int flags)
 EXPORT_SYMBOL_GPL(dma_buf_fd);
 
 /**
- * dma_buf_get - returns the dma_buf structure related to an fd
- * @fd:	[in]	fd associated with the dma_buf to be returned
+ * dma_buf_get - returns the struct dma_buf related to an fd
+ * @fd:	[in]	fd associated with the struct dma_buf to be returned
  *
- * On success, returns the dma_buf structure associated with an fd; uses
+ * On success, returns the struct dma_buf associated with an fd; uses
  * file's refcounting done by fget to increase refcount. returns ERR_PTR
  * otherwise.
  */
@@ -653,8 +654,7 @@ void dma_buf_put(struct dma_buf *dmabuf)
 EXPORT_SYMBOL_GPL(dma_buf_put);
 
 /**
- * dma_buf_dynamic_attach - Add the device to dma_buf's attachments list; optionally,
- * calls attach() of dma_buf_ops to allow device-specific attach functionality
+ * dma_buf_dynamic_attach - Add the device to dma_buf's attachments list
  * @dmabuf:		[in]	buffer to attach device to.
  * @dev:		[in]	device to be attached.
  * @importer_ops:	[in]	importer operations for the attachment
@@ -663,6 +663,9 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
  * Returns struct dma_buf_attachment pointer for this attachment. Attachments
  * must be cleaned up by calling dma_buf_detach().
  *
+ * Optionally this calls &dma_buf_ops.attach to allow device-specific attach
+ * functionality.
+ *
  * Returns:
  *
  * A pointer to newly created &dma_buf_attachment on success, or a negative
@@ -769,12 +772,13 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
 EXPORT_SYMBOL_GPL(dma_buf_attach);
 
 /**
- * dma_buf_detach - Remove the given attachment from dmabuf's attachments list;
- * optionally calls detach() of dma_buf_ops for device-specific detach
+ * dma_buf_detach - Remove the given attachment from dmabuf's attachments list
  * @dmabuf:	[in]	buffer to detach from.
  * @attach:	[in]	attachment to be detached; is free'd after this call.
  *
  * Clean up a device attachment obtained by calling dma_buf_attach().
+ *
+ * Optionally this calls &dma_buf_ops.detach for device-specific detach.
  */
 void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
 {
@@ -1061,11 +1065,12 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
  *   shootdowns would increase the complexity quite a bit.
  *
  *   Interface::
+ *
  *      int dma_buf_mmap(struct dma_buf \*, struct vm_area_struct \*,
  *		       unsigned long);
  *
  *   If the importing subsystem simply provides a special-purpose mmap call to
- *   set up a mapping in userspace, calling do_mmap with dma_buf->file will
+ *   set up a mapping in userspace, calling do_mmap with &dma_buf.file will
  *   equally achieve that for a dma-buf object.
  */
 
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 7eca37c8b10c..43802a31b25d 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -85,10 +85,10 @@ struct dma_buf_ops {
 	/**
 	 * @pin:
 	 *
-	 * This is called by dma_buf_pin and lets the exporter know that the
+	 * This is called by dma_buf_pin() and lets the exporter know that the
 	 * DMA-buf can't be moved any more.
 	 *
-	 * This is called with the dmabuf->resv object locked and is mutual
+	 * This is called with the &dmabuf.resv object locked and is mutual
 	 * exclusive with @cache_sgt_mapping.
 	 *
 	 * This callback is optional and should only be used in limited use
@@ -103,7 +103,7 @@ struct dma_buf_ops {
 	/**
 	 * @unpin:
 	 *
-	 * This is called by dma_buf_unpin and lets the exporter know that the
+	 * This is called by dma_buf_unpin() and lets the exporter know that the
 	 * DMA-buf can be moved again.
 	 *
 	 * This is called with the dmabuf->resv object locked and is mutual
-- 
2.29.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 3/4] dma-buf: begin/end_cpu might lock the dma_resv lock
  2020-12-11 15:58 ` Daniel Vetter
  (?)
@ 2020-12-11 15:58   ` Daniel Vetter
  -1 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-11 15:58 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Thomas Zimmermann, Sumit Semwal, Christian König,
	linux-media, linaro-mm-sig

At least amdgpu and i915 do, so lets just document this as the rule.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index e1fa6c6f02c4..00d5afe904cc 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1118,6 +1118,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
 	if (WARN_ON(!dmabuf))
 		return -EINVAL;
 
+	might_lock(&dma_buf->resv.lock);
+
 	if (dmabuf->ops->begin_cpu_access)
 		ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
 
@@ -1151,6 +1153,8 @@ int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
 
 	WARN_ON(!dmabuf);
 
+	might_lock(&dma_buf->resv.lock);
+
 	if (dmabuf->ops->end_cpu_access)
 		ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 3/4] dma-buf: begin/end_cpu might lock the dma_resv lock
@ 2020-12-11 15:58   ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-11 15:58 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, linaro-mm-sig,
	Thomas Zimmermann, Daniel Vetter, Christian König,
	linux-media

At least amdgpu and i915 do, so lets just document this as the rule.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index e1fa6c6f02c4..00d5afe904cc 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1118,6 +1118,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
 	if (WARN_ON(!dmabuf))
 		return -EINVAL;
 
+	might_lock(&dma_buf->resv.lock);
+
 	if (dmabuf->ops->begin_cpu_access)
 		ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
 
@@ -1151,6 +1153,8 @@ int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
 
 	WARN_ON(!dmabuf);
 
+	might_lock(&dma_buf->resv.lock);
+
 	if (dmabuf->ops->end_cpu_access)
 		ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
 
-- 
2.29.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [Intel-gfx] [PATCH 3/4] dma-buf: begin/end_cpu might lock the dma_resv lock
@ 2020-12-11 15:58   ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-11 15:58 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Sumit Semwal,
	linaro-mm-sig, Thomas Zimmermann, Daniel Vetter,
	Christian König, linux-media

At least amdgpu and i915 do, so lets just document this as the rule.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index e1fa6c6f02c4..00d5afe904cc 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1118,6 +1118,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
 	if (WARN_ON(!dmabuf))
 		return -EINVAL;
 
+	might_lock(&dma_buf->resv.lock);
+
 	if (dmabuf->ops->begin_cpu_access)
 		ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
 
@@ -1151,6 +1153,8 @@ int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
 
 	WARN_ON(!dmabuf);
 
+	might_lock(&dma_buf->resv.lock);
+
 	if (dmabuf->ops->end_cpu_access)
 		ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
 
-- 
2.29.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 4/4] dma-buf: doc polish for pin/unpin
  2020-12-11 15:58 ` Daniel Vetter
  (?)
@ 2020-12-11 15:58   ` Daniel Vetter
  -1 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-11 15:58 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Thomas Zimmermann, Sumit Semwal, Christian König,
	linux-media, linaro-mm-sig

Motivated by a discussion with Christian and Thomas: Try to untangle a
bit what's relevant for importers and what's relevant for exporters.

Also add an assert that really only dynamic importers use the api
function, anything else doesn't make sense.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 19 ++++++++++++++++---
 include/linux/dma-buf.h   |  8 +++++---
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 00d5afe904cc..1065545e9ca1 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -809,9 +809,15 @@ EXPORT_SYMBOL_GPL(dma_buf_detach);
 
 /**
  * dma_buf_pin - Lock down the DMA-buf
- *
  * @attach:	[in]	attachment which should be pinned
  *
+ * Only dynamic importers (who set up @attach with dma_buf_dynamic_attach()) may
+ * call this, and only for limited use cases like scanout and not for temporary
+ * pin operations. It is not permitted to allow userspace to pin arbitrary
+ * amounts of buffers through this interface.
+ *
+ * Buffers must be unpinned by calling dma_buf_unpin().
+ *
  * Returns:
  * 0 on success, negative error code on failure.
  */
@@ -820,6 +826,8 @@ int dma_buf_pin(struct dma_buf_attachment *attach)
 	struct dma_buf *dmabuf = attach->dmabuf;
 	int ret = 0;
 
+	WARN_ON(!dma_buf_attachment_is_dynamic(attach));
+
 	dma_resv_assert_held(dmabuf->resv);
 
 	if (dmabuf->ops->pin)
@@ -830,14 +838,19 @@ int dma_buf_pin(struct dma_buf_attachment *attach)
 EXPORT_SYMBOL_GPL(dma_buf_pin);
 
 /**
- * dma_buf_unpin - Remove lock from DMA-buf
- *
+ * dma_buf_unpin - Unpin a DMA-buf
  * @attach:	[in]	attachment which should be unpinned
+ *
+ * This unpins a buffer pinned by dma_buf_pin() and allows the exporter to move
+ * any mapping of @attach again and inform the importer through
+ * &dma_buf_attach_ops.move_notify.
  */
 void dma_buf_unpin(struct dma_buf_attachment *attach)
 {
 	struct dma_buf *dmabuf = attach->dmabuf;
 
+	WARN_ON(!dma_buf_attachment_is_dynamic(attach));
+
 	dma_resv_assert_held(dmabuf->resv);
 
 	if (dmabuf->ops->unpin)
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 43802a31b25d..628681bf6c99 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -86,13 +86,15 @@ struct dma_buf_ops {
 	 * @pin:
 	 *
 	 * This is called by dma_buf_pin() and lets the exporter know that the
-	 * DMA-buf can't be moved any more.
+	 * DMA-buf can't be moved any more. The exporter should pin the buffer
+	 * into system memory to make sure it is generally accessible by other
+	 * devices.
 	 *
 	 * This is called with the &dmabuf.resv object locked and is mutual
 	 * exclusive with @cache_sgt_mapping.
 	 *
-	 * This callback is optional and should only be used in limited use
-	 * cases like scanout and not for temporary pin operations.
+	 * This is called automatically for non-dynamic importers from
+	 * dma_buf_attach().
 	 *
 	 * Returns:
 	 *
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH 4/4] dma-buf: doc polish for pin/unpin
@ 2020-12-11 15:58   ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-11 15:58 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, linaro-mm-sig,
	Thomas Zimmermann, Daniel Vetter, Christian König,
	linux-media

Motivated by a discussion with Christian and Thomas: Try to untangle a
bit what's relevant for importers and what's relevant for exporters.

Also add an assert that really only dynamic importers use the api
function, anything else doesn't make sense.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 19 ++++++++++++++++---
 include/linux/dma-buf.h   |  8 +++++---
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 00d5afe904cc..1065545e9ca1 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -809,9 +809,15 @@ EXPORT_SYMBOL_GPL(dma_buf_detach);
 
 /**
  * dma_buf_pin - Lock down the DMA-buf
- *
  * @attach:	[in]	attachment which should be pinned
  *
+ * Only dynamic importers (who set up @attach with dma_buf_dynamic_attach()) may
+ * call this, and only for limited use cases like scanout and not for temporary
+ * pin operations. It is not permitted to allow userspace to pin arbitrary
+ * amounts of buffers through this interface.
+ *
+ * Buffers must be unpinned by calling dma_buf_unpin().
+ *
  * Returns:
  * 0 on success, negative error code on failure.
  */
@@ -820,6 +826,8 @@ int dma_buf_pin(struct dma_buf_attachment *attach)
 	struct dma_buf *dmabuf = attach->dmabuf;
 	int ret = 0;
 
+	WARN_ON(!dma_buf_attachment_is_dynamic(attach));
+
 	dma_resv_assert_held(dmabuf->resv);
 
 	if (dmabuf->ops->pin)
@@ -830,14 +838,19 @@ int dma_buf_pin(struct dma_buf_attachment *attach)
 EXPORT_SYMBOL_GPL(dma_buf_pin);
 
 /**
- * dma_buf_unpin - Remove lock from DMA-buf
- *
+ * dma_buf_unpin - Unpin a DMA-buf
  * @attach:	[in]	attachment which should be unpinned
+ *
+ * This unpins a buffer pinned by dma_buf_pin() and allows the exporter to move
+ * any mapping of @attach again and inform the importer through
+ * &dma_buf_attach_ops.move_notify.
  */
 void dma_buf_unpin(struct dma_buf_attachment *attach)
 {
 	struct dma_buf *dmabuf = attach->dmabuf;
 
+	WARN_ON(!dma_buf_attachment_is_dynamic(attach));
+
 	dma_resv_assert_held(dmabuf->resv);
 
 	if (dmabuf->ops->unpin)
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 43802a31b25d..628681bf6c99 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -86,13 +86,15 @@ struct dma_buf_ops {
 	 * @pin:
 	 *
 	 * This is called by dma_buf_pin() and lets the exporter know that the
-	 * DMA-buf can't be moved any more.
+	 * DMA-buf can't be moved any more. The exporter should pin the buffer
+	 * into system memory to make sure it is generally accessible by other
+	 * devices.
 	 *
 	 * This is called with the &dmabuf.resv object locked and is mutual
 	 * exclusive with @cache_sgt_mapping.
 	 *
-	 * This callback is optional and should only be used in limited use
-	 * cases like scanout and not for temporary pin operations.
+	 * This is called automatically for non-dynamic importers from
+	 * dma_buf_attach().
 	 *
 	 * Returns:
 	 *
-- 
2.29.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [Intel-gfx] [PATCH 4/4] dma-buf: doc polish for pin/unpin
@ 2020-12-11 15:58   ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-11 15:58 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Sumit Semwal,
	linaro-mm-sig, Thomas Zimmermann, Daniel Vetter,
	Christian König, linux-media

Motivated by a discussion with Christian and Thomas: Try to untangle a
bit what's relevant for importers and what's relevant for exporters.

Also add an assert that really only dynamic importers use the api
function, anything else doesn't make sense.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 19 ++++++++++++++++---
 include/linux/dma-buf.h   |  8 +++++---
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 00d5afe904cc..1065545e9ca1 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -809,9 +809,15 @@ EXPORT_SYMBOL_GPL(dma_buf_detach);
 
 /**
  * dma_buf_pin - Lock down the DMA-buf
- *
  * @attach:	[in]	attachment which should be pinned
  *
+ * Only dynamic importers (who set up @attach with dma_buf_dynamic_attach()) may
+ * call this, and only for limited use cases like scanout and not for temporary
+ * pin operations. It is not permitted to allow userspace to pin arbitrary
+ * amounts of buffers through this interface.
+ *
+ * Buffers must be unpinned by calling dma_buf_unpin().
+ *
  * Returns:
  * 0 on success, negative error code on failure.
  */
@@ -820,6 +826,8 @@ int dma_buf_pin(struct dma_buf_attachment *attach)
 	struct dma_buf *dmabuf = attach->dmabuf;
 	int ret = 0;
 
+	WARN_ON(!dma_buf_attachment_is_dynamic(attach));
+
 	dma_resv_assert_held(dmabuf->resv);
 
 	if (dmabuf->ops->pin)
@@ -830,14 +838,19 @@ int dma_buf_pin(struct dma_buf_attachment *attach)
 EXPORT_SYMBOL_GPL(dma_buf_pin);
 
 /**
- * dma_buf_unpin - Remove lock from DMA-buf
- *
+ * dma_buf_unpin - Unpin a DMA-buf
  * @attach:	[in]	attachment which should be unpinned
+ *
+ * This unpins a buffer pinned by dma_buf_pin() and allows the exporter to move
+ * any mapping of @attach again and inform the importer through
+ * &dma_buf_attach_ops.move_notify.
  */
 void dma_buf_unpin(struct dma_buf_attachment *attach)
 {
 	struct dma_buf *dmabuf = attach->dmabuf;
 
+	WARN_ON(!dma_buf_attachment_is_dynamic(attach));
+
 	dma_resv_assert_held(dmabuf->resv);
 
 	if (dmabuf->ops->unpin)
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 43802a31b25d..628681bf6c99 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -86,13 +86,15 @@ struct dma_buf_ops {
 	 * @pin:
 	 *
 	 * This is called by dma_buf_pin() and lets the exporter know that the
-	 * DMA-buf can't be moved any more.
+	 * DMA-buf can't be moved any more. The exporter should pin the buffer
+	 * into system memory to make sure it is generally accessible by other
+	 * devices.
 	 *
 	 * This is called with the &dmabuf.resv object locked and is mutual
 	 * exclusive with @cache_sgt_mapping.
 	 *
-	 * This callback is optional and should only be used in limited use
-	 * cases like scanout and not for temporary pin operations.
+	 * This is called automatically for non-dynamic importers from
+	 * dma_buf_attach().
 	 *
 	 * Returns:
 	 *
-- 
2.29.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges
  2020-12-11 15:58 ` Daniel Vetter
                   ` (4 preceding siblings ...)
  (?)
@ 2020-12-11 16:21 ` Patchwork
  -1 siblings, 0 replies; 44+ messages in thread
From: Patchwork @ 2020-12-11 16:21 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges
URL   : https://patchwork.freedesktop.org/series/84849/
State : failure

== Summary ==

CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  DESCEND  objtool
  CHK     include/generated/compile.h
  CC      drivers/dma-buf/dma-buf.o
In file included from ./include/linux/kernel.h:15,
                 from ./include/linux/list.h:9,
                 from ./include/linux/wait.h:7,
                 from ./include/linux/wait_bit.h:8,
                 from ./include/linux/fs.h:6,
                 from drivers/dma-buf/dma-buf.c:14:
drivers/dma-buf/dma-buf.c: In function ‘dma_buf_begin_cpu_access’:
drivers/dma-buf/dma-buf.c:1134:14: error: ‘dma_buf’ undeclared (first use in this function)
  might_lock(&dma_buf->resv.lock);
              ^~~~~~~
./include/linux/typecheck.h:11:9: note: in definition of macro ‘typecheck’
  typeof(x) __dummy2; \
         ^
drivers/dma-buf/dma-buf.c:1134:2: note: in expansion of macro ‘might_lock’
  might_lock(&dma_buf->resv.lock);
  ^~~~~~~~~~
drivers/dma-buf/dma-buf.c:1134:14: note: each undeclared identifier is reported only once for each function it appears in
  might_lock(&dma_buf->resv.lock);
              ^~~~~~~
./include/linux/typecheck.h:11:9: note: in definition of macro ‘typecheck’
  typeof(x) __dummy2; \
         ^
drivers/dma-buf/dma-buf.c:1134:2: note: in expansion of macro ‘might_lock’
  might_lock(&dma_buf->resv.lock);
  ^~~~~~~~~~
./include/linux/typecheck.h:12:18: warning: comparison of distinct pointer types lacks a cast
  (void)(&__dummy == &__dummy2); \
                  ^~
./include/linux/lockdep.h:542:2: note: in expansion of macro ‘typecheck’
  typecheck(struct lockdep_map *, &(lock)->dep_map);  \
  ^~~~~~~~~
drivers/dma-buf/dma-buf.c:1134:2: note: in expansion of macro ‘might_lock’
  might_lock(&dma_buf->resv.lock);
  ^~~~~~~~~~
drivers/dma-buf/dma-buf.c: In function ‘dma_buf_end_cpu_access’:
drivers/dma-buf/dma-buf.c:1169:14: error: ‘dma_buf’ undeclared (first use in this function)
  might_lock(&dma_buf->resv.lock);
              ^~~~~~~
./include/linux/typecheck.h:11:9: note: in definition of macro ‘typecheck’
  typeof(x) __dummy2; \
         ^
drivers/dma-buf/dma-buf.c:1169:2: note: in expansion of macro ‘might_lock’
  might_lock(&dma_buf->resv.lock);
  ^~~~~~~~~~
./include/linux/typecheck.h:12:18: warning: comparison of distinct pointer types lacks a cast
  (void)(&__dummy == &__dummy2); \
                  ^~
./include/linux/lockdep.h:542:2: note: in expansion of macro ‘typecheck’
  typecheck(struct lockdep_map *, &(lock)->dep_map);  \
  ^~~~~~~~~
drivers/dma-buf/dma-buf.c:1169:2: note: in expansion of macro ‘might_lock’
  might_lock(&dma_buf->resv.lock);
  ^~~~~~~~~~
scripts/Makefile.build:279: recipe for target 'drivers/dma-buf/dma-buf.o' failed
make[2]: *** [drivers/dma-buf/dma-buf.o] Error 1
scripts/Makefile.build:496: recipe for target 'drivers/dma-buf' failed
make[1]: *** [drivers/dma-buf] Error 2
Makefile:1805: recipe for target 'drivers' failed
make: *** [drivers] Error 2


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 3/4] dma-buf: begin/end_cpu might lock the dma_resv lock
  2020-12-11 15:58   ` Daniel Vetter
  (?)
  (?)
@ 2020-12-11 22:31     ` kernel test robot
  -1 siblings, 0 replies; 44+ messages in thread
From: kernel test robot @ 2020-12-11 22:31 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: kbuild-all, clang-built-linux, Daniel Vetter,
	Intel Graphics Development, linaro-mm-sig, Thomas Zimmermann,
	Christian König, linux-media

[-- Attachment #1: Type: text/plain, Size: 4371 bytes --]

Hi Daniel,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20201211]
[also build test ERROR on v5.10-rc7]
[cannot apply to tegra-drm/drm/tegra/for-next linus/master v5.10-rc7 v5.10-rc6 v5.10-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daniel-Vetter/dma-buf-Remove-kmap-kerneldoc-vestiges/20201212-000040
base:    3cc2bd440f2171f093b3a8480a4b54d8c270ed38
config: powerpc64-randconfig-r035-20201210 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5ff35356f1af2bb92785b38c657463924d9ec386)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc64 cross compiling tool for clang build
        # apt-get install binutils-powerpc64-linux-gnu
        # https://github.com/0day-ci/linux/commit/24d4fcf0e09c80974556c7639cb630f86a544378
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daniel-Vetter/dma-buf-Remove-kmap-kerneldoc-vestiges/20201212-000040
        git checkout 24d4fcf0e09c80974556c7639cb630f86a544378
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/dma-buf/dma-buf.c:1121:14: error: use of undeclared identifier 'dma_buf'
           might_lock(&dma_buf->resv.lock);
                       ^
>> drivers/dma-buf/dma-buf.c:1121:14: error: use of undeclared identifier 'dma_buf'
>> drivers/dma-buf/dma-buf.c:1121:14: error: use of undeclared identifier 'dma_buf'
   drivers/dma-buf/dma-buf.c:1156:14: error: use of undeclared identifier 'dma_buf'
           might_lock(&dma_buf->resv.lock);
                       ^
   drivers/dma-buf/dma-buf.c:1156:14: error: use of undeclared identifier 'dma_buf'
   drivers/dma-buf/dma-buf.c:1156:14: error: use of undeclared identifier 'dma_buf'
   6 errors generated.

vim +/dma_buf +1121 drivers/dma-buf/dma-buf.c

  1093	
  1094	/**
  1095	 * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
  1096	 * cpu in the kernel context. Calls begin_cpu_access to allow exporter-specific
  1097	 * preparations. Coherency is only guaranteed in the specified range for the
  1098	 * specified access direction.
  1099	 * @dmabuf:	[in]	buffer to prepare cpu access for.
  1100	 * @direction:	[in]	length of range for cpu access.
  1101	 *
  1102	 * After the cpu access is complete the caller should call
  1103	 * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
  1104	 * it guaranteed to be coherent with other DMA access.
  1105	 *
  1106	 * This function will also wait for any DMA transactions tracked through
  1107	 * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
  1108	 * synchronization this function will only ensure cache coherency, callers must
  1109	 * ensure synchronization with such DMA transactions on their own.
  1110	 *
  1111	 * Can return negative error values, returns 0 on success.
  1112	 */
  1113	int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
  1114				     enum dma_data_direction direction)
  1115	{
  1116		int ret = 0;
  1117	
  1118		if (WARN_ON(!dmabuf))
  1119			return -EINVAL;
  1120	
> 1121		might_lock(&dma_buf->resv.lock);
  1122	
  1123		if (dmabuf->ops->begin_cpu_access)
  1124			ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
  1125	
  1126		/* Ensure that all fences are waited upon - but we first allow
  1127		 * the native handler the chance to do so more efficiently if it
  1128		 * chooses. A double invocation here will be reasonably cheap no-op.
  1129		 */
  1130		if (ret == 0)
  1131			ret = __dma_buf_begin_cpu_access(dmabuf, direction);
  1132	
  1133		return ret;
  1134	}
  1135	EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
  1136	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 44099 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 3/4] dma-buf: begin/end_cpu might lock the dma_resv lock
@ 2020-12-11 22:31     ` kernel test robot
  0 siblings, 0 replies; 44+ messages in thread
From: kernel test robot @ 2020-12-11 22:31 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: kbuild-all, Daniel Vetter, Intel Graphics Development,
	linaro-mm-sig, clang-built-linux, Thomas Zimmermann,
	Christian König, linux-media

[-- Attachment #1: Type: text/plain, Size: 4371 bytes --]

Hi Daniel,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20201211]
[also build test ERROR on v5.10-rc7]
[cannot apply to tegra-drm/drm/tegra/for-next linus/master v5.10-rc7 v5.10-rc6 v5.10-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daniel-Vetter/dma-buf-Remove-kmap-kerneldoc-vestiges/20201212-000040
base:    3cc2bd440f2171f093b3a8480a4b54d8c270ed38
config: powerpc64-randconfig-r035-20201210 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5ff35356f1af2bb92785b38c657463924d9ec386)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc64 cross compiling tool for clang build
        # apt-get install binutils-powerpc64-linux-gnu
        # https://github.com/0day-ci/linux/commit/24d4fcf0e09c80974556c7639cb630f86a544378
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daniel-Vetter/dma-buf-Remove-kmap-kerneldoc-vestiges/20201212-000040
        git checkout 24d4fcf0e09c80974556c7639cb630f86a544378
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/dma-buf/dma-buf.c:1121:14: error: use of undeclared identifier 'dma_buf'
           might_lock(&dma_buf->resv.lock);
                       ^
>> drivers/dma-buf/dma-buf.c:1121:14: error: use of undeclared identifier 'dma_buf'
>> drivers/dma-buf/dma-buf.c:1121:14: error: use of undeclared identifier 'dma_buf'
   drivers/dma-buf/dma-buf.c:1156:14: error: use of undeclared identifier 'dma_buf'
           might_lock(&dma_buf->resv.lock);
                       ^
   drivers/dma-buf/dma-buf.c:1156:14: error: use of undeclared identifier 'dma_buf'
   drivers/dma-buf/dma-buf.c:1156:14: error: use of undeclared identifier 'dma_buf'
   6 errors generated.

vim +/dma_buf +1121 drivers/dma-buf/dma-buf.c

  1093	
  1094	/**
  1095	 * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
  1096	 * cpu in the kernel context. Calls begin_cpu_access to allow exporter-specific
  1097	 * preparations. Coherency is only guaranteed in the specified range for the
  1098	 * specified access direction.
  1099	 * @dmabuf:	[in]	buffer to prepare cpu access for.
  1100	 * @direction:	[in]	length of range for cpu access.
  1101	 *
  1102	 * After the cpu access is complete the caller should call
  1103	 * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
  1104	 * it guaranteed to be coherent with other DMA access.
  1105	 *
  1106	 * This function will also wait for any DMA transactions tracked through
  1107	 * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
  1108	 * synchronization this function will only ensure cache coherency, callers must
  1109	 * ensure synchronization with such DMA transactions on their own.
  1110	 *
  1111	 * Can return negative error values, returns 0 on success.
  1112	 */
  1113	int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
  1114				     enum dma_data_direction direction)
  1115	{
  1116		int ret = 0;
  1117	
  1118		if (WARN_ON(!dmabuf))
  1119			return -EINVAL;
  1120	
> 1121		might_lock(&dma_buf->resv.lock);
  1122	
  1123		if (dmabuf->ops->begin_cpu_access)
  1124			ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
  1125	
  1126		/* Ensure that all fences are waited upon - but we first allow
  1127		 * the native handler the chance to do so more efficiently if it
  1128		 * chooses. A double invocation here will be reasonably cheap no-op.
  1129		 */
  1130		if (ret == 0)
  1131			ret = __dma_buf_begin_cpu_access(dmabuf, direction);
  1132	
  1133		return ret;
  1134	}
  1135	EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
  1136	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 44099 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Intel-gfx] [PATCH 3/4] dma-buf: begin/end_cpu might lock the dma_resv lock
@ 2020-12-11 22:31     ` kernel test robot
  0 siblings, 0 replies; 44+ messages in thread
From: kernel test robot @ 2020-12-11 22:31 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: kbuild-all, Daniel Vetter, Intel Graphics Development,
	linaro-mm-sig, clang-built-linux, Thomas Zimmermann,
	Christian König, linux-media

[-- Attachment #1: Type: text/plain, Size: 4371 bytes --]

Hi Daniel,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20201211]
[also build test ERROR on v5.10-rc7]
[cannot apply to tegra-drm/drm/tegra/for-next linus/master v5.10-rc7 v5.10-rc6 v5.10-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daniel-Vetter/dma-buf-Remove-kmap-kerneldoc-vestiges/20201212-000040
base:    3cc2bd440f2171f093b3a8480a4b54d8c270ed38
config: powerpc64-randconfig-r035-20201210 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5ff35356f1af2bb92785b38c657463924d9ec386)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc64 cross compiling tool for clang build
        # apt-get install binutils-powerpc64-linux-gnu
        # https://github.com/0day-ci/linux/commit/24d4fcf0e09c80974556c7639cb630f86a544378
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daniel-Vetter/dma-buf-Remove-kmap-kerneldoc-vestiges/20201212-000040
        git checkout 24d4fcf0e09c80974556c7639cb630f86a544378
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/dma-buf/dma-buf.c:1121:14: error: use of undeclared identifier 'dma_buf'
           might_lock(&dma_buf->resv.lock);
                       ^
>> drivers/dma-buf/dma-buf.c:1121:14: error: use of undeclared identifier 'dma_buf'
>> drivers/dma-buf/dma-buf.c:1121:14: error: use of undeclared identifier 'dma_buf'
   drivers/dma-buf/dma-buf.c:1156:14: error: use of undeclared identifier 'dma_buf'
           might_lock(&dma_buf->resv.lock);
                       ^
   drivers/dma-buf/dma-buf.c:1156:14: error: use of undeclared identifier 'dma_buf'
   drivers/dma-buf/dma-buf.c:1156:14: error: use of undeclared identifier 'dma_buf'
   6 errors generated.

vim +/dma_buf +1121 drivers/dma-buf/dma-buf.c

  1093	
  1094	/**
  1095	 * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
  1096	 * cpu in the kernel context. Calls begin_cpu_access to allow exporter-specific
  1097	 * preparations. Coherency is only guaranteed in the specified range for the
  1098	 * specified access direction.
  1099	 * @dmabuf:	[in]	buffer to prepare cpu access for.
  1100	 * @direction:	[in]	length of range for cpu access.
  1101	 *
  1102	 * After the cpu access is complete the caller should call
  1103	 * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
  1104	 * it guaranteed to be coherent with other DMA access.
  1105	 *
  1106	 * This function will also wait for any DMA transactions tracked through
  1107	 * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
  1108	 * synchronization this function will only ensure cache coherency, callers must
  1109	 * ensure synchronization with such DMA transactions on their own.
  1110	 *
  1111	 * Can return negative error values, returns 0 on success.
  1112	 */
  1113	int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
  1114				     enum dma_data_direction direction)
  1115	{
  1116		int ret = 0;
  1117	
  1118		if (WARN_ON(!dmabuf))
  1119			return -EINVAL;
  1120	
> 1121		might_lock(&dma_buf->resv.lock);
  1122	
  1123		if (dmabuf->ops->begin_cpu_access)
  1124			ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
  1125	
  1126		/* Ensure that all fences are waited upon - but we first allow
  1127		 * the native handler the chance to do so more efficiently if it
  1128		 * chooses. A double invocation here will be reasonably cheap no-op.
  1129		 */
  1130		if (ret == 0)
  1131			ret = __dma_buf_begin_cpu_access(dmabuf, direction);
  1132	
  1133		return ret;
  1134	}
  1135	EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
  1136	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 44099 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 3/4] dma-buf: begin/end_cpu might lock the dma_resv lock
@ 2020-12-11 22:31     ` kernel test robot
  0 siblings, 0 replies; 44+ messages in thread
From: kernel test robot @ 2020-12-11 22:31 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4467 bytes --]

Hi Daniel,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20201211]
[also build test ERROR on v5.10-rc7]
[cannot apply to tegra-drm/drm/tegra/for-next linus/master v5.10-rc7 v5.10-rc6 v5.10-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daniel-Vetter/dma-buf-Remove-kmap-kerneldoc-vestiges/20201212-000040
base:    3cc2bd440f2171f093b3a8480a4b54d8c270ed38
config: powerpc64-randconfig-r035-20201210 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5ff35356f1af2bb92785b38c657463924d9ec386)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc64 cross compiling tool for clang build
        # apt-get install binutils-powerpc64-linux-gnu
        # https://github.com/0day-ci/linux/commit/24d4fcf0e09c80974556c7639cb630f86a544378
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daniel-Vetter/dma-buf-Remove-kmap-kerneldoc-vestiges/20201212-000040
        git checkout 24d4fcf0e09c80974556c7639cb630f86a544378
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/dma-buf/dma-buf.c:1121:14: error: use of undeclared identifier 'dma_buf'
           might_lock(&dma_buf->resv.lock);
                       ^
>> drivers/dma-buf/dma-buf.c:1121:14: error: use of undeclared identifier 'dma_buf'
>> drivers/dma-buf/dma-buf.c:1121:14: error: use of undeclared identifier 'dma_buf'
   drivers/dma-buf/dma-buf.c:1156:14: error: use of undeclared identifier 'dma_buf'
           might_lock(&dma_buf->resv.lock);
                       ^
   drivers/dma-buf/dma-buf.c:1156:14: error: use of undeclared identifier 'dma_buf'
   drivers/dma-buf/dma-buf.c:1156:14: error: use of undeclared identifier 'dma_buf'
   6 errors generated.

vim +/dma_buf +1121 drivers/dma-buf/dma-buf.c

  1093	
  1094	/**
  1095	 * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the
  1096	 * cpu in the kernel context. Calls begin_cpu_access to allow exporter-specific
  1097	 * preparations. Coherency is only guaranteed in the specified range for the
  1098	 * specified access direction.
  1099	 * @dmabuf:	[in]	buffer to prepare cpu access for.
  1100	 * @direction:	[in]	length of range for cpu access.
  1101	 *
  1102	 * After the cpu access is complete the caller should call
  1103	 * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
  1104	 * it guaranteed to be coherent with other DMA access.
  1105	 *
  1106	 * This function will also wait for any DMA transactions tracked through
  1107	 * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
  1108	 * synchronization this function will only ensure cache coherency, callers must
  1109	 * ensure synchronization with such DMA transactions on their own.
  1110	 *
  1111	 * Can return negative error values, returns 0 on success.
  1112	 */
  1113	int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
  1114				     enum dma_data_direction direction)
  1115	{
  1116		int ret = 0;
  1117	
  1118		if (WARN_ON(!dmabuf))
  1119			return -EINVAL;
  1120	
> 1121		might_lock(&dma_buf->resv.lock);
  1122	
  1123		if (dmabuf->ops->begin_cpu_access)
  1124			ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
  1125	
  1126		/* Ensure that all fences are waited upon - but we first allow
  1127		 * the native handler the chance to do so more efficiently if it
  1128		 * chooses. A double invocation here will be reasonably cheap no-op.
  1129		 */
  1130		if (ret == 0)
  1131			ret = __dma_buf_begin_cpu_access(dmabuf, direction);
  1132	
  1133		return ret;
  1134	}
  1135	EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access);
  1136	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 44099 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
  2020-12-11 15:58 ` Daniel Vetter
  (?)
@ 2020-12-14 10:33   ` Christian König
  -1 siblings, 0 replies; 44+ messages in thread
From: Christian König @ 2020-12-14 10:33 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Thomas Zimmermann,
	Sumit Semwal, linux-media, linaro-mm-sig

Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> Also try to clarify a bit when dma_buf_begin/end_cpu_access should
> be called.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
> ---
>   drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
>   include/linux/dma-buf.h   | 25 +++++++++----------------
>   2 files changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index e63684d4cd90..a12fdffa130f 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
>    *   vmalloc space might be limited and result in vmap calls failing.
>    *
>    *   Interfaces::
> + *
>    *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
>    *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
>    *
>    *   The vmap call can fail if there is no vmap support in the exporter, or if
> - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
> - *   that the dma-buf layer keeps a reference count for all vmap access and
> - *   calls down into the exporter's vmap function only when no vmapping exists,
> - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
> - *   provided by taking the dma_buf->lock mutex.
> + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
> + *   count for all vmap access and calls down into the exporter's vmap function
> + *   only when no vmapping exists, and only unmaps it once. Protection against
> + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.

Who is talking the lock? The caller of the dma_buf_vmap/vunmap() 
functions, the functions itself or the callback inside the exporter?

Christian.

>    *
>    * - For full compatibility on the importer side with existing userspace
>    *   interfaces, which might already support mmap'ing buffers. This is needed in
> @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>    * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
>    * it guaranteed to be coherent with other DMA access.
>    *
> + * This function will also wait for any DMA transactions tracked through
> + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
> + * synchronization this function will only ensure cache coherency, callers must
> + * ensure synchronization with such DMA transactions on their own.
> + *
>    * Can return negative error values, returns 0 on success.
>    */
>   int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
>    * This call may fail due to lack of virtual mapping address space.
>    * These calls are optional in drivers. The intended use for them
>    * is for mapping objects linear in kernel space for high use objects.
> - * Please attempt to use kmap/kunmap before thinking about these interfaces.
> + *
> + * To ensure coherency users must call dma_buf_begin_cpu_access() and
> + * dma_buf_end_cpu_access() around any cpu access performed through this
> + * mapping.
>    *
>    * Returns 0 on success, or a negative errno code otherwise.
>    */
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index cf72699cb2bc..7eca37c8b10c 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -183,24 +183,19 @@ struct dma_buf_ops {
>   	 * @begin_cpu_access:
>   	 *
>   	 * This is called from dma_buf_begin_cpu_access() and allows the
> -	 * exporter to ensure that the memory is actually available for cpu
> -	 * access - the exporter might need to allocate or swap-in and pin the
> -	 * backing storage. The exporter also needs to ensure that cpu access is
> -	 * coherent for the access direction. The direction can be used by the
> -	 * exporter to optimize the cache flushing, i.e. access with a different
> +	 * exporter to ensure that the memory is actually coherent for cpu
> +	 * access. The exporter also needs to ensure that cpu access is coherent
> +	 * for the access direction. The direction can be used by the exporter
> +	 * to optimize the cache flushing, i.e. access with a different
>   	 * direction (read instead of write) might return stale or even bogus
>   	 * data (e.g. when the exporter needs to copy the data to temporary
>   	 * storage).
>   	 *
> -	 * This callback is optional.
> +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
> +	 * command for userspace mappings established through @mmap, and also
> +	 * for kernel mappings established with @vmap.
>   	 *
> -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
> -	 * from userspace (where storage shouldn't be pinned to avoid handing
> -	 * de-factor mlock rights to userspace) and for the kernel-internal
> -	 * users of the various kmap interfaces, where the backing storage must
> -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
> -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
> -	 * real problem.
> +	 * This callback is optional.
>   	 *
>   	 * Returns:
>   	 *
> @@ -216,9 +211,7 @@ struct dma_buf_ops {
>   	 *
>   	 * This is called from dma_buf_end_cpu_access() when the importer is
>   	 * done accessing the CPU. The exporter can use this to flush caches and
> -	 * unpin any resources pinned in @begin_cpu_access.
> -	 * The result of any dma_buf kmap calls after end_cpu_access is
> -	 * undefined.
> +	 * undo anything else done in @begin_cpu_access.
>   	 *
>   	 * This callback is optional.
>   	 *


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
@ 2020-12-14 10:33   ` Christian König
  0 siblings, 0 replies; 44+ messages in thread
From: Christian König @ 2020-12-14 10:33 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Intel Graphics Development, linaro-mm-sig, Thomas Zimmermann,
	Daniel Vetter, linux-media

Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> Also try to clarify a bit when dma_buf_begin/end_cpu_access should
> be called.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
> ---
>   drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
>   include/linux/dma-buf.h   | 25 +++++++++----------------
>   2 files changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index e63684d4cd90..a12fdffa130f 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
>    *   vmalloc space might be limited and result in vmap calls failing.
>    *
>    *   Interfaces::
> + *
>    *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
>    *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
>    *
>    *   The vmap call can fail if there is no vmap support in the exporter, or if
> - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
> - *   that the dma-buf layer keeps a reference count for all vmap access and
> - *   calls down into the exporter's vmap function only when no vmapping exists,
> - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
> - *   provided by taking the dma_buf->lock mutex.
> + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
> + *   count for all vmap access and calls down into the exporter's vmap function
> + *   only when no vmapping exists, and only unmaps it once. Protection against
> + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.

Who is talking the lock? The caller of the dma_buf_vmap/vunmap() 
functions, the functions itself or the callback inside the exporter?

Christian.

>    *
>    * - For full compatibility on the importer side with existing userspace
>    *   interfaces, which might already support mmap'ing buffers. This is needed in
> @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>    * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
>    * it guaranteed to be coherent with other DMA access.
>    *
> + * This function will also wait for any DMA transactions tracked through
> + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
> + * synchronization this function will only ensure cache coherency, callers must
> + * ensure synchronization with such DMA transactions on their own.
> + *
>    * Can return negative error values, returns 0 on success.
>    */
>   int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
>    * This call may fail due to lack of virtual mapping address space.
>    * These calls are optional in drivers. The intended use for them
>    * is for mapping objects linear in kernel space for high use objects.
> - * Please attempt to use kmap/kunmap before thinking about these interfaces.
> + *
> + * To ensure coherency users must call dma_buf_begin_cpu_access() and
> + * dma_buf_end_cpu_access() around any cpu access performed through this
> + * mapping.
>    *
>    * Returns 0 on success, or a negative errno code otherwise.
>    */
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index cf72699cb2bc..7eca37c8b10c 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -183,24 +183,19 @@ struct dma_buf_ops {
>   	 * @begin_cpu_access:
>   	 *
>   	 * This is called from dma_buf_begin_cpu_access() and allows the
> -	 * exporter to ensure that the memory is actually available for cpu
> -	 * access - the exporter might need to allocate or swap-in and pin the
> -	 * backing storage. The exporter also needs to ensure that cpu access is
> -	 * coherent for the access direction. The direction can be used by the
> -	 * exporter to optimize the cache flushing, i.e. access with a different
> +	 * exporter to ensure that the memory is actually coherent for cpu
> +	 * access. The exporter also needs to ensure that cpu access is coherent
> +	 * for the access direction. The direction can be used by the exporter
> +	 * to optimize the cache flushing, i.e. access with a different
>   	 * direction (read instead of write) might return stale or even bogus
>   	 * data (e.g. when the exporter needs to copy the data to temporary
>   	 * storage).
>   	 *
> -	 * This callback is optional.
> +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
> +	 * command for userspace mappings established through @mmap, and also
> +	 * for kernel mappings established with @vmap.
>   	 *
> -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
> -	 * from userspace (where storage shouldn't be pinned to avoid handing
> -	 * de-factor mlock rights to userspace) and for the kernel-internal
> -	 * users of the various kmap interfaces, where the backing storage must
> -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
> -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
> -	 * real problem.
> +	 * This callback is optional.
>   	 *
>   	 * Returns:
>   	 *
> @@ -216,9 +211,7 @@ struct dma_buf_ops {
>   	 *
>   	 * This is called from dma_buf_end_cpu_access() when the importer is
>   	 * done accessing the CPU. The exporter can use this to flush caches and
> -	 * unpin any resources pinned in @begin_cpu_access.
> -	 * The result of any dma_buf kmap calls after end_cpu_access is
> -	 * undefined.
> +	 * undo anything else done in @begin_cpu_access.
>   	 *
>   	 * This callback is optional.
>   	 *

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Intel-gfx] [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
@ 2020-12-14 10:33   ` Christian König
  0 siblings, 0 replies; 44+ messages in thread
From: Christian König @ 2020-12-14 10:33 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Intel Graphics Development, linaro-mm-sig, Thomas Zimmermann,
	Daniel Vetter, Sumit Semwal, linux-media

Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> Also try to clarify a bit when dma_buf_begin/end_cpu_access should
> be called.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
> ---
>   drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
>   include/linux/dma-buf.h   | 25 +++++++++----------------
>   2 files changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index e63684d4cd90..a12fdffa130f 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
>    *   vmalloc space might be limited and result in vmap calls failing.
>    *
>    *   Interfaces::
> + *
>    *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
>    *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
>    *
>    *   The vmap call can fail if there is no vmap support in the exporter, or if
> - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
> - *   that the dma-buf layer keeps a reference count for all vmap access and
> - *   calls down into the exporter's vmap function only when no vmapping exists,
> - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
> - *   provided by taking the dma_buf->lock mutex.
> + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
> + *   count for all vmap access and calls down into the exporter's vmap function
> + *   only when no vmapping exists, and only unmaps it once. Protection against
> + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.

Who is talking the lock? The caller of the dma_buf_vmap/vunmap() 
functions, the functions itself or the callback inside the exporter?

Christian.

>    *
>    * - For full compatibility on the importer side with existing userspace
>    *   interfaces, which might already support mmap'ing buffers. This is needed in
> @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>    * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
>    * it guaranteed to be coherent with other DMA access.
>    *
> + * This function will also wait for any DMA transactions tracked through
> + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
> + * synchronization this function will only ensure cache coherency, callers must
> + * ensure synchronization with such DMA transactions on their own.
> + *
>    * Can return negative error values, returns 0 on success.
>    */
>   int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
>    * This call may fail due to lack of virtual mapping address space.
>    * These calls are optional in drivers. The intended use for them
>    * is for mapping objects linear in kernel space for high use objects.
> - * Please attempt to use kmap/kunmap before thinking about these interfaces.
> + *
> + * To ensure coherency users must call dma_buf_begin_cpu_access() and
> + * dma_buf_end_cpu_access() around any cpu access performed through this
> + * mapping.
>    *
>    * Returns 0 on success, or a negative errno code otherwise.
>    */
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index cf72699cb2bc..7eca37c8b10c 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -183,24 +183,19 @@ struct dma_buf_ops {
>   	 * @begin_cpu_access:
>   	 *
>   	 * This is called from dma_buf_begin_cpu_access() and allows the
> -	 * exporter to ensure that the memory is actually available for cpu
> -	 * access - the exporter might need to allocate or swap-in and pin the
> -	 * backing storage. The exporter also needs to ensure that cpu access is
> -	 * coherent for the access direction. The direction can be used by the
> -	 * exporter to optimize the cache flushing, i.e. access with a different
> +	 * exporter to ensure that the memory is actually coherent for cpu
> +	 * access. The exporter also needs to ensure that cpu access is coherent
> +	 * for the access direction. The direction can be used by the exporter
> +	 * to optimize the cache flushing, i.e. access with a different
>   	 * direction (read instead of write) might return stale or even bogus
>   	 * data (e.g. when the exporter needs to copy the data to temporary
>   	 * storage).
>   	 *
> -	 * This callback is optional.
> +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
> +	 * command for userspace mappings established through @mmap, and also
> +	 * for kernel mappings established with @vmap.
>   	 *
> -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
> -	 * from userspace (where storage shouldn't be pinned to avoid handing
> -	 * de-factor mlock rights to userspace) and for the kernel-internal
> -	 * users of the various kmap interfaces, where the backing storage must
> -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
> -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
> -	 * real problem.
> +	 * This callback is optional.
>   	 *
>   	 * Returns:
>   	 *
> @@ -216,9 +211,7 @@ struct dma_buf_ops {
>   	 *
>   	 * This is called from dma_buf_end_cpu_access() when the importer is
>   	 * done accessing the CPU. The exporter can use this to flush caches and
> -	 * unpin any resources pinned in @begin_cpu_access.
> -	 * The result of any dma_buf kmap calls after end_cpu_access is
> -	 * undefined.
> +	 * undo anything else done in @begin_cpu_access.
>   	 *
>   	 * This callback is optional.
>   	 *

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 3/4] dma-buf: begin/end_cpu might lock the dma_resv lock
  2020-12-11 15:58   ` Daniel Vetter
  (?)
@ 2020-12-14 10:33     ` Christian König
  -1 siblings, 0 replies; 44+ messages in thread
From: Christian König @ 2020-12-14 10:33 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Thomas Zimmermann,
	Sumit Semwal, linux-media, linaro-mm-sig

Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> At least amdgpu and i915 do, so lets just document this as the rule.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/dma-buf/dma-buf.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index e1fa6c6f02c4..00d5afe904cc 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1118,6 +1118,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>   	if (WARN_ON(!dmabuf))
>   		return -EINVAL;
>   
> +	might_lock(&dma_buf->resv.lock);
> +
>   	if (dmabuf->ops->begin_cpu_access)
>   		ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
>   
> @@ -1151,6 +1153,8 @@ int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>   
>   	WARN_ON(!dmabuf);
>   
> +	might_lock(&dma_buf->resv.lock);
> +
>   	if (dmabuf->ops->end_cpu_access)
>   		ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
>   


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 3/4] dma-buf: begin/end_cpu might lock the dma_resv lock
@ 2020-12-14 10:33     ` Christian König
  0 siblings, 0 replies; 44+ messages in thread
From: Christian König @ 2020-12-14 10:33 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Intel Graphics Development, linaro-mm-sig, Thomas Zimmermann,
	Daniel Vetter, linux-media

Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> At least amdgpu and i915 do, so lets just document this as the rule.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/dma-buf/dma-buf.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index e1fa6c6f02c4..00d5afe904cc 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1118,6 +1118,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>   	if (WARN_ON(!dmabuf))
>   		return -EINVAL;
>   
> +	might_lock(&dma_buf->resv.lock);
> +
>   	if (dmabuf->ops->begin_cpu_access)
>   		ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
>   
> @@ -1151,6 +1153,8 @@ int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>   
>   	WARN_ON(!dmabuf);
>   
> +	might_lock(&dma_buf->resv.lock);
> +
>   	if (dmabuf->ops->end_cpu_access)
>   		ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
>   

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Intel-gfx] [PATCH 3/4] dma-buf: begin/end_cpu might lock the dma_resv lock
@ 2020-12-14 10:33     ` Christian König
  0 siblings, 0 replies; 44+ messages in thread
From: Christian König @ 2020-12-14 10:33 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Intel Graphics Development, linaro-mm-sig, Thomas Zimmermann,
	Daniel Vetter, Sumit Semwal, linux-media

Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> At least amdgpu and i915 do, so lets just document this as the rule.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/dma-buf/dma-buf.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index e1fa6c6f02c4..00d5afe904cc 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1118,6 +1118,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>   	if (WARN_ON(!dmabuf))
>   		return -EINVAL;
>   
> +	might_lock(&dma_buf->resv.lock);
> +
>   	if (dmabuf->ops->begin_cpu_access)
>   		ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
>   
> @@ -1151,6 +1153,8 @@ int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>   
>   	WARN_ON(!dmabuf);
>   
> +	might_lock(&dma_buf->resv.lock);
> +
>   	if (dmabuf->ops->end_cpu_access)
>   		ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
>   

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
  2020-12-14 10:33   ` Christian König
  (?)
@ 2020-12-14 16:01     ` Daniel Vetter
  -1 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-14 16:01 UTC (permalink / raw)
  To: Christian König
  Cc: Daniel Vetter, DRI Development, Intel Graphics Development,
	Daniel Vetter, Thomas Zimmermann, Sumit Semwal, linux-media,
	linaro-mm-sig

On Mon, Dec 14, 2020 at 11:33:10AM +0100, Christian König wrote:
> Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> > Also try to clarify a bit when dma_buf_begin/end_cpu_access should
> > be called.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: linux-media@vger.kernel.org
> > Cc: linaro-mm-sig@lists.linaro.org
> > ---
> >   drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
> >   include/linux/dma-buf.h   | 25 +++++++++----------------
> >   2 files changed, 23 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index e63684d4cd90..a12fdffa130f 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
> >    *   vmalloc space might be limited and result in vmap calls failing.
> >    *
> >    *   Interfaces::
> > + *
> >    *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
> >    *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
> >    *
> >    *   The vmap call can fail if there is no vmap support in the exporter, or if
> > - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
> > - *   that the dma-buf layer keeps a reference count for all vmap access and
> > - *   calls down into the exporter's vmap function only when no vmapping exists,
> > - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
> > - *   provided by taking the dma_buf->lock mutex.
> > + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
> > + *   count for all vmap access and calls down into the exporter's vmap function
> > + *   only when no vmapping exists, and only unmaps it once. Protection against
> > + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
> 
> Who is talking the lock? The caller of the dma_buf_vmap/vunmap() functions,
> the functions itself or the callback inside the exporter?

That's the part I didn't change at all here, just re-laid out the line
breaking. I only removed the outdated kmap section here.

Should I do another patch and remove this one sentence here (it's kinda
pointless and generally we don't muse about implementation details that
callers don't care about)?

I did try and do a cursory review of the dma-buf docs, but this is kinda
not meant as an all-out revamp. Just a few things I've noticed while
reviewing Thomas' vmap_local stuff.
-Daniel

> 
> Christian.
> 
> >    *
> >    * - For full compatibility on the importer side with existing userspace
> >    *   interfaces, which might already support mmap'ing buffers. This is needed in
> > @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> >    * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
> >    * it guaranteed to be coherent with other DMA access.
> >    *
> > + * This function will also wait for any DMA transactions tracked through
> > + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
> > + * synchronization this function will only ensure cache coherency, callers must
> > + * ensure synchronization with such DMA transactions on their own.
> > + *
> >    * Can return negative error values, returns 0 on success.
> >    */
> >   int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
> >    * This call may fail due to lack of virtual mapping address space.
> >    * These calls are optional in drivers. The intended use for them
> >    * is for mapping objects linear in kernel space for high use objects.
> > - * Please attempt to use kmap/kunmap before thinking about these interfaces.
> > + *
> > + * To ensure coherency users must call dma_buf_begin_cpu_access() and
> > + * dma_buf_end_cpu_access() around any cpu access performed through this
> > + * mapping.
> >    *
> >    * Returns 0 on success, or a negative errno code otherwise.
> >    */
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index cf72699cb2bc..7eca37c8b10c 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -183,24 +183,19 @@ struct dma_buf_ops {
> >   	 * @begin_cpu_access:
> >   	 *
> >   	 * This is called from dma_buf_begin_cpu_access() and allows the
> > -	 * exporter to ensure that the memory is actually available for cpu
> > -	 * access - the exporter might need to allocate or swap-in and pin the
> > -	 * backing storage. The exporter also needs to ensure that cpu access is
> > -	 * coherent for the access direction. The direction can be used by the
> > -	 * exporter to optimize the cache flushing, i.e. access with a different
> > +	 * exporter to ensure that the memory is actually coherent for cpu
> > +	 * access. The exporter also needs to ensure that cpu access is coherent
> > +	 * for the access direction. The direction can be used by the exporter
> > +	 * to optimize the cache flushing, i.e. access with a different
> >   	 * direction (read instead of write) might return stale or even bogus
> >   	 * data (e.g. when the exporter needs to copy the data to temporary
> >   	 * storage).
> >   	 *
> > -	 * This callback is optional.
> > +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
> > +	 * command for userspace mappings established through @mmap, and also
> > +	 * for kernel mappings established with @vmap.
> >   	 *
> > -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
> > -	 * from userspace (where storage shouldn't be pinned to avoid handing
> > -	 * de-factor mlock rights to userspace) and for the kernel-internal
> > -	 * users of the various kmap interfaces, where the backing storage must
> > -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
> > -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
> > -	 * real problem.
> > +	 * This callback is optional.
> >   	 *
> >   	 * Returns:
> >   	 *
> > @@ -216,9 +211,7 @@ struct dma_buf_ops {
> >   	 *
> >   	 * This is called from dma_buf_end_cpu_access() when the importer is
> >   	 * done accessing the CPU. The exporter can use this to flush caches and
> > -	 * unpin any resources pinned in @begin_cpu_access.
> > -	 * The result of any dma_buf kmap calls after end_cpu_access is
> > -	 * undefined.
> > +	 * undo anything else done in @begin_cpu_access.
> >   	 *
> >   	 * This callback is optional.
> >   	 *
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
@ 2020-12-14 16:01     ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-14 16:01 UTC (permalink / raw)
  To: Christian König
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	linaro-mm-sig, Thomas Zimmermann, Daniel Vetter, linux-media

On Mon, Dec 14, 2020 at 11:33:10AM +0100, Christian König wrote:
> Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> > Also try to clarify a bit when dma_buf_begin/end_cpu_access should
> > be called.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: linux-media@vger.kernel.org
> > Cc: linaro-mm-sig@lists.linaro.org
> > ---
> >   drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
> >   include/linux/dma-buf.h   | 25 +++++++++----------------
> >   2 files changed, 23 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index e63684d4cd90..a12fdffa130f 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
> >    *   vmalloc space might be limited and result in vmap calls failing.
> >    *
> >    *   Interfaces::
> > + *
> >    *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
> >    *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
> >    *
> >    *   The vmap call can fail if there is no vmap support in the exporter, or if
> > - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
> > - *   that the dma-buf layer keeps a reference count for all vmap access and
> > - *   calls down into the exporter's vmap function only when no vmapping exists,
> > - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
> > - *   provided by taking the dma_buf->lock mutex.
> > + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
> > + *   count for all vmap access and calls down into the exporter's vmap function
> > + *   only when no vmapping exists, and only unmaps it once. Protection against
> > + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
> 
> Who is talking the lock? The caller of the dma_buf_vmap/vunmap() functions,
> the functions itself or the callback inside the exporter?

That's the part I didn't change at all here, just re-laid out the line
breaking. I only removed the outdated kmap section here.

Should I do another patch and remove this one sentence here (it's kinda
pointless and generally we don't muse about implementation details that
callers don't care about)?

I did try and do a cursory review of the dma-buf docs, but this is kinda
not meant as an all-out revamp. Just a few things I've noticed while
reviewing Thomas' vmap_local stuff.
-Daniel

> 
> Christian.
> 
> >    *
> >    * - For full compatibility on the importer side with existing userspace
> >    *   interfaces, which might already support mmap'ing buffers. This is needed in
> > @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> >    * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
> >    * it guaranteed to be coherent with other DMA access.
> >    *
> > + * This function will also wait for any DMA transactions tracked through
> > + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
> > + * synchronization this function will only ensure cache coherency, callers must
> > + * ensure synchronization with such DMA transactions on their own.
> > + *
> >    * Can return negative error values, returns 0 on success.
> >    */
> >   int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
> >    * This call may fail due to lack of virtual mapping address space.
> >    * These calls are optional in drivers. The intended use for them
> >    * is for mapping objects linear in kernel space for high use objects.
> > - * Please attempt to use kmap/kunmap before thinking about these interfaces.
> > + *
> > + * To ensure coherency users must call dma_buf_begin_cpu_access() and
> > + * dma_buf_end_cpu_access() around any cpu access performed through this
> > + * mapping.
> >    *
> >    * Returns 0 on success, or a negative errno code otherwise.
> >    */
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index cf72699cb2bc..7eca37c8b10c 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -183,24 +183,19 @@ struct dma_buf_ops {
> >   	 * @begin_cpu_access:
> >   	 *
> >   	 * This is called from dma_buf_begin_cpu_access() and allows the
> > -	 * exporter to ensure that the memory is actually available for cpu
> > -	 * access - the exporter might need to allocate or swap-in and pin the
> > -	 * backing storage. The exporter also needs to ensure that cpu access is
> > -	 * coherent for the access direction. The direction can be used by the
> > -	 * exporter to optimize the cache flushing, i.e. access with a different
> > +	 * exporter to ensure that the memory is actually coherent for cpu
> > +	 * access. The exporter also needs to ensure that cpu access is coherent
> > +	 * for the access direction. The direction can be used by the exporter
> > +	 * to optimize the cache flushing, i.e. access with a different
> >   	 * direction (read instead of write) might return stale or even bogus
> >   	 * data (e.g. when the exporter needs to copy the data to temporary
> >   	 * storage).
> >   	 *
> > -	 * This callback is optional.
> > +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
> > +	 * command for userspace mappings established through @mmap, and also
> > +	 * for kernel mappings established with @vmap.
> >   	 *
> > -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
> > -	 * from userspace (where storage shouldn't be pinned to avoid handing
> > -	 * de-factor mlock rights to userspace) and for the kernel-internal
> > -	 * users of the various kmap interfaces, where the backing storage must
> > -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
> > -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
> > -	 * real problem.
> > +	 * This callback is optional.
> >   	 *
> >   	 * Returns:
> >   	 *
> > @@ -216,9 +211,7 @@ struct dma_buf_ops {
> >   	 *
> >   	 * This is called from dma_buf_end_cpu_access() when the importer is
> >   	 * done accessing the CPU. The exporter can use this to flush caches and
> > -	 * unpin any resources pinned in @begin_cpu_access.
> > -	 * The result of any dma_buf kmap calls after end_cpu_access is
> > -	 * undefined.
> > +	 * undo anything else done in @begin_cpu_access.
> >   	 *
> >   	 * This callback is optional.
> >   	 *
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Intel-gfx] [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
@ 2020-12-14 16:01     ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-14 16:01 UTC (permalink / raw)
  To: Christian König
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	linaro-mm-sig, Thomas Zimmermann, Daniel Vetter, Sumit Semwal,
	linux-media

On Mon, Dec 14, 2020 at 11:33:10AM +0100, Christian König wrote:
> Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> > Also try to clarify a bit when dma_buf_begin/end_cpu_access should
> > be called.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: linux-media@vger.kernel.org
> > Cc: linaro-mm-sig@lists.linaro.org
> > ---
> >   drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
> >   include/linux/dma-buf.h   | 25 +++++++++----------------
> >   2 files changed, 23 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index e63684d4cd90..a12fdffa130f 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
> >    *   vmalloc space might be limited and result in vmap calls failing.
> >    *
> >    *   Interfaces::
> > + *
> >    *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
> >    *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
> >    *
> >    *   The vmap call can fail if there is no vmap support in the exporter, or if
> > - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
> > - *   that the dma-buf layer keeps a reference count for all vmap access and
> > - *   calls down into the exporter's vmap function only when no vmapping exists,
> > - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
> > - *   provided by taking the dma_buf->lock mutex.
> > + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
> > + *   count for all vmap access and calls down into the exporter's vmap function
> > + *   only when no vmapping exists, and only unmaps it once. Protection against
> > + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
> 
> Who is talking the lock? The caller of the dma_buf_vmap/vunmap() functions,
> the functions itself or the callback inside the exporter?

That's the part I didn't change at all here, just re-laid out the line
breaking. I only removed the outdated kmap section here.

Should I do another patch and remove this one sentence here (it's kinda
pointless and generally we don't muse about implementation details that
callers don't care about)?

I did try and do a cursory review of the dma-buf docs, but this is kinda
not meant as an all-out revamp. Just a few things I've noticed while
reviewing Thomas' vmap_local stuff.
-Daniel

> 
> Christian.
> 
> >    *
> >    * - For full compatibility on the importer side with existing userspace
> >    *   interfaces, which might already support mmap'ing buffers. This is needed in
> > @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> >    * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
> >    * it guaranteed to be coherent with other DMA access.
> >    *
> > + * This function will also wait for any DMA transactions tracked through
> > + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
> > + * synchronization this function will only ensure cache coherency, callers must
> > + * ensure synchronization with such DMA transactions on their own.
> > + *
> >    * Can return negative error values, returns 0 on success.
> >    */
> >   int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
> >    * This call may fail due to lack of virtual mapping address space.
> >    * These calls are optional in drivers. The intended use for them
> >    * is for mapping objects linear in kernel space for high use objects.
> > - * Please attempt to use kmap/kunmap before thinking about these interfaces.
> > + *
> > + * To ensure coherency users must call dma_buf_begin_cpu_access() and
> > + * dma_buf_end_cpu_access() around any cpu access performed through this
> > + * mapping.
> >    *
> >    * Returns 0 on success, or a negative errno code otherwise.
> >    */
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index cf72699cb2bc..7eca37c8b10c 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -183,24 +183,19 @@ struct dma_buf_ops {
> >   	 * @begin_cpu_access:
> >   	 *
> >   	 * This is called from dma_buf_begin_cpu_access() and allows the
> > -	 * exporter to ensure that the memory is actually available for cpu
> > -	 * access - the exporter might need to allocate or swap-in and pin the
> > -	 * backing storage. The exporter also needs to ensure that cpu access is
> > -	 * coherent for the access direction. The direction can be used by the
> > -	 * exporter to optimize the cache flushing, i.e. access with a different
> > +	 * exporter to ensure that the memory is actually coherent for cpu
> > +	 * access. The exporter also needs to ensure that cpu access is coherent
> > +	 * for the access direction. The direction can be used by the exporter
> > +	 * to optimize the cache flushing, i.e. access with a different
> >   	 * direction (read instead of write) might return stale or even bogus
> >   	 * data (e.g. when the exporter needs to copy the data to temporary
> >   	 * storage).
> >   	 *
> > -	 * This callback is optional.
> > +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
> > +	 * command for userspace mappings established through @mmap, and also
> > +	 * for kernel mappings established with @vmap.
> >   	 *
> > -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
> > -	 * from userspace (where storage shouldn't be pinned to avoid handing
> > -	 * de-factor mlock rights to userspace) and for the kernel-internal
> > -	 * users of the various kmap interfaces, where the backing storage must
> > -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
> > -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
> > -	 * real problem.
> > +	 * This callback is optional.
> >   	 *
> >   	 * Returns:
> >   	 *
> > @@ -216,9 +211,7 @@ struct dma_buf_ops {
> >   	 *
> >   	 * This is called from dma_buf_end_cpu_access() when the importer is
> >   	 * done accessing the CPU. The exporter can use this to flush caches and
> > -	 * unpin any resources pinned in @begin_cpu_access.
> > -	 * The result of any dma_buf kmap calls after end_cpu_access is
> > -	 * undefined.
> > +	 * undo anything else done in @begin_cpu_access.
> >   	 *
> >   	 * This callback is optional.
> >   	 *
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH] dma-buf: begin/end_cpu might lock the dma_resv lock
  2020-12-11 15:58   ` Daniel Vetter
  (?)
@ 2020-12-14 17:16     ` Daniel Vetter
  -1 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-14 17:16 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Thomas Zimmermann, Sumit Semwal, Christian König,
	linux-media, linaro-mm-sig

At least amdgpu and i915 do, so lets just document this as the rule.

v2: Works better with less typos (intel-gfx-ci)

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index e1fa6c6f02c4..a0a02ef888da 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1118,6 +1118,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
 	if (WARN_ON(!dmabuf))
 		return -EINVAL;
 
+	might_lock(&dmabuf->resv->lock.base);
+
 	if (dmabuf->ops->begin_cpu_access)
 		ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
 
@@ -1151,6 +1153,8 @@ int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
 
 	WARN_ON(!dmabuf);
 
+	might_lock(&dmabuf->resv->lock.base);
+
 	if (dmabuf->ops->end_cpu_access)
 		ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH] dma-buf: begin/end_cpu might lock the dma_resv lock
@ 2020-12-14 17:16     ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-14 17:16 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, linaro-mm-sig,
	Thomas Zimmermann, Daniel Vetter, Christian König,
	linux-media

At least amdgpu and i915 do, so lets just document this as the rule.

v2: Works better with less typos (intel-gfx-ci)

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index e1fa6c6f02c4..a0a02ef888da 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1118,6 +1118,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
 	if (WARN_ON(!dmabuf))
 		return -EINVAL;
 
+	might_lock(&dmabuf->resv->lock.base);
+
 	if (dmabuf->ops->begin_cpu_access)
 		ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
 
@@ -1151,6 +1153,8 @@ int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
 
 	WARN_ON(!dmabuf);
 
+	might_lock(&dmabuf->resv->lock.base);
+
 	if (dmabuf->ops->end_cpu_access)
 		ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
 
-- 
2.29.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [Intel-gfx] [PATCH] dma-buf: begin/end_cpu might lock the dma_resv lock
@ 2020-12-14 17:16     ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-14 17:16 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Sumit Semwal,
	linaro-mm-sig, Thomas Zimmermann, Daniel Vetter,
	Christian König, linux-media

At least amdgpu and i915 do, so lets just document this as the rule.

v2: Works better with less typos (intel-gfx-ci)

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-buf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index e1fa6c6f02c4..a0a02ef888da 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1118,6 +1118,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
 	if (WARN_ON(!dmabuf))
 		return -EINVAL;
 
+	might_lock(&dmabuf->resv->lock.base);
+
 	if (dmabuf->ops->begin_cpu_access)
 		ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
 
@@ -1151,6 +1153,8 @@ int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
 
 	WARN_ON(!dmabuf);
 
+	might_lock(&dmabuf->resv->lock.base);
+
 	if (dmabuf->ops->end_cpu_access)
 		ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
 
-- 
2.29.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev2)
  2020-12-11 15:58 ` Daniel Vetter
                   ` (6 preceding siblings ...)
  (?)
@ 2020-12-14 18:02 ` Patchwork
  -1 siblings, 0 replies; 44+ messages in thread
From: Patchwork @ 2020-12-14 18:02 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev2)
URL   : https://patchwork.freedesktop.org/series/84849/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
431d5d802932 dma-buf: Remove kmap kerneldoc vestiges
-:115: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 84 lines checked
23e5090ffe7c dma-buf: some kerneldoc formatting fixes
-:144: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 108 lines checked
019147465c7d dma-buf: begin/end_cpu might lock the dma_resv lock
-:41: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 16 lines checked
a0a86fd27dc7 dma-buf: doc polish for pin/unpin
-:96: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 63 lines checked


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [Intel-gfx] ✗ Fi.CI.DOCS: warning for series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev2)
  2020-12-11 15:58 ` Daniel Vetter
                   ` (7 preceding siblings ...)
  (?)
@ 2020-12-14 18:06 ` Patchwork
  -1 siblings, 0 replies; 44+ messages in thread
From: Patchwork @ 2020-12-14 18:06 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev2)
URL   : https://patchwork.freedesktop.org/series/84849/
State : warning

== Summary ==

$ make htmldocs 2>&1 > /dev/null | grep i915
Error: Cannot open file ./drivers/gpu/drm/i915/gt/intel_lrc.c
WARNING: kernel-doc './scripts/kernel-doc -rst -enable-lineno -sphinx-version 1.7.9 -function Logical Rings, Logical Ring Contexts and Execlists ./drivers/gpu/drm/i915/gt/intel_lrc.c' failed with return code 1


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev2)
  2020-12-11 15:58 ` Daniel Vetter
                   ` (8 preceding siblings ...)
  (?)
@ 2020-12-14 18:33 ` Patchwork
  -1 siblings, 0 replies; 44+ messages in thread
From: Patchwork @ 2020-12-14 18:33 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 4790 bytes --]

== Series Details ==

Series: series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev2)
URL   : https://patchwork.freedesktop.org/series/84849/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_9481 -> Patchwork_19138
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_19138 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_19138, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19138/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_19138:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_sync@basic-each:
    - fi-kbl-guc:         [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9481/fi-kbl-guc/igt@gem_sync@basic-each.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19138/fi-kbl-guc/igt@gem_sync@basic-each.html

  
Known issues
------------

  Here are the changes found in Patchwork_19138 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s0:
    - fi-tgl-u2:          [PASS][3] -> [FAIL][4] ([i915#1888])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9481/fi-tgl-u2/igt@gem_exec_suspend@basic-s0.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19138/fi-tgl-u2/igt@gem_exec_suspend@basic-s0.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - fi-snb-2600:        NOTRUN -> [SKIP][5] ([fdo#109271]) +30 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19138/fi-snb-2600/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-snb-2600:        NOTRUN -> [SKIP][6] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19138/fi-snb-2600/igt@kms_chamelium@hdmi-crc-fast.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-tgl-y:           [PASS][7] -> [DMESG-WARN][8] ([i915#402]) +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9481/fi-tgl-y/igt@prime_vgem@basic-fence-flip.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19138/fi-tgl-y/igt@prime_vgem@basic-fence-flip.html

  * igt@runner@aborted:
    - fi-kbl-guc:         NOTRUN -> [FAIL][9] ([i915#2295] / [i915#2722])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19138/fi-kbl-guc/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-snb-2600:        [DMESG-WARN][10] -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9481/fi-snb-2600/igt@gem_exec_suspend@basic-s3.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19138/fi-snb-2600/igt@gem_exec_suspend@basic-s3.html

  * igt@prime_self_import@basic-with_one_bo_two_files:
    - fi-tgl-y:           [DMESG-WARN][12] ([i915#402]) -> [PASS][13] +1 similar issue
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9481/fi-tgl-y/igt@prime_self_import@basic-with_one_bo_two_files.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19138/fi-tgl-y/igt@prime_self_import@basic-with_one_bo_two_files.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#2295]: https://gitlab.freedesktop.org/drm/intel/issues/2295
  [i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402


Participating hosts (44 -> 39)
------------------------------

  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_9481 -> Patchwork_19138

  CI-20190529: 20190529
  CI_DRM_9481: 968d6b8835487575d68732a7c0a1588795900de4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5893: ec4073d30b428aaf199c0f4de6c0a5ebdc1c1c65 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19138: a0a86fd27dc7516dcd0e3e8d9f98f1f20335349b @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a0a86fd27dc7 dma-buf: doc polish for pin/unpin
019147465c7d dma-buf: begin/end_cpu might lock the dma_resv lock
23e5090ffe7c dma-buf: some kerneldoc formatting fixes
431d5d802932 dma-buf: Remove kmap kerneldoc vestiges

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19138/index.html

[-- Attachment #1.2: Type: text/html, Size: 5749 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] dma-buf: begin/end_cpu might lock the dma_resv lock
  2020-12-14 17:16     ` Daniel Vetter
  (?)
@ 2020-12-15 14:16       ` Christian König
  -1 siblings, 0 replies; 44+ messages in thread
From: Christian König @ 2020-12-15 14:16 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Thomas Zimmermann,
	Sumit Semwal, linux-media, linaro-mm-sig

Am 14.12.20 um 18:16 schrieb Daniel Vetter:
> At least amdgpu and i915 do, so lets just document this as the rule.
>
> v2: Works better with less typos (intel-gfx-ci)
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/dma-buf/dma-buf.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index e1fa6c6f02c4..a0a02ef888da 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1118,6 +1118,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>   	if (WARN_ON(!dmabuf))
>   		return -EINVAL;
>   
> +	might_lock(&dmabuf->resv->lock.base);
> +
>   	if (dmabuf->ops->begin_cpu_access)
>   		ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
>   
> @@ -1151,6 +1153,8 @@ int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>   
>   	WARN_ON(!dmabuf);
>   
> +	might_lock(&dmabuf->resv->lock.base);
> +
>   	if (dmabuf->ops->end_cpu_access)
>   		ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
>   


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH] dma-buf: begin/end_cpu might lock the dma_resv lock
@ 2020-12-15 14:16       ` Christian König
  0 siblings, 0 replies; 44+ messages in thread
From: Christian König @ 2020-12-15 14:16 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Intel Graphics Development, linaro-mm-sig, Thomas Zimmermann,
	Daniel Vetter, linux-media

Am 14.12.20 um 18:16 schrieb Daniel Vetter:
> At least amdgpu and i915 do, so lets just document this as the rule.
>
> v2: Works better with less typos (intel-gfx-ci)
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/dma-buf/dma-buf.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index e1fa6c6f02c4..a0a02ef888da 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1118,6 +1118,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>   	if (WARN_ON(!dmabuf))
>   		return -EINVAL;
>   
> +	might_lock(&dmabuf->resv->lock.base);
> +
>   	if (dmabuf->ops->begin_cpu_access)
>   		ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
>   
> @@ -1151,6 +1153,8 @@ int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>   
>   	WARN_ON(!dmabuf);
>   
> +	might_lock(&dmabuf->resv->lock.base);
> +
>   	if (dmabuf->ops->end_cpu_access)
>   		ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
>   

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Intel-gfx] [PATCH] dma-buf: begin/end_cpu might lock the dma_resv lock
@ 2020-12-15 14:16       ` Christian König
  0 siblings, 0 replies; 44+ messages in thread
From: Christian König @ 2020-12-15 14:16 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Intel Graphics Development, linaro-mm-sig, Thomas Zimmermann,
	Daniel Vetter, Sumit Semwal, linux-media

Am 14.12.20 um 18:16 schrieb Daniel Vetter:
> At least amdgpu and i915 do, so lets just document this as the rule.
>
> v2: Works better with less typos (intel-gfx-ci)
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/dma-buf/dma-buf.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index e1fa6c6f02c4..a0a02ef888da 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1118,6 +1118,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>   	if (WARN_ON(!dmabuf))
>   		return -EINVAL;
>   
> +	might_lock(&dmabuf->resv->lock.base);
> +
>   	if (dmabuf->ops->begin_cpu_access)
>   		ret = dmabuf->ops->begin_cpu_access(dmabuf, direction);
>   
> @@ -1151,6 +1153,8 @@ int dma_buf_end_cpu_access(struct dma_buf *dmabuf,
>   
>   	WARN_ON(!dmabuf);
>   
> +	might_lock(&dmabuf->resv->lock.base);
> +
>   	if (dmabuf->ops->end_cpu_access)
>   		ret = dmabuf->ops->end_cpu_access(dmabuf, direction);
>   

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
  2020-12-14 16:01     ` Daniel Vetter
  (?)
@ 2020-12-15 14:18       ` Christian König
  -1 siblings, 0 replies; 44+ messages in thread
From: Christian König @ 2020-12-15 14:18 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Vetter, DRI Development, Intel Graphics Development,
	Daniel Vetter, Thomas Zimmermann, Sumit Semwal, linux-media,
	linaro-mm-sig

Am 14.12.20 um 17:01 schrieb Daniel Vetter:
> On Mon, Dec 14, 2020 at 11:33:10AM +0100, Christian König wrote:
>> Am 11.12.20 um 16:58 schrieb Daniel Vetter:
>>> Also try to clarify a bit when dma_buf_begin/end_cpu_access should
>>> be called.
>>>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>> Cc: Sumit Semwal <sumit.semwal@linaro.org>
>>> Cc: "Christian König" <christian.koenig@amd.com>
>>> Cc: linux-media@vger.kernel.org
>>> Cc: linaro-mm-sig@lists.linaro.org
>>> ---
>>>    drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
>>>    include/linux/dma-buf.h   | 25 +++++++++----------------
>>>    2 files changed, 23 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index e63684d4cd90..a12fdffa130f 100644
>>> --- a/drivers/dma-buf/dma-buf.c
>>> +++ b/drivers/dma-buf/dma-buf.c
>>> @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
>>>     *   vmalloc space might be limited and result in vmap calls failing.
>>>     *
>>>     *   Interfaces::
>>> + *
>>>     *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
>>>     *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
>>>     *
>>>     *   The vmap call can fail if there is no vmap support in the exporter, or if
>>> - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
>>> - *   that the dma-buf layer keeps a reference count for all vmap access and
>>> - *   calls down into the exporter's vmap function only when no vmapping exists,
>>> - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
>>> - *   provided by taking the dma_buf->lock mutex.
>>> + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
>>> + *   count for all vmap access and calls down into the exporter's vmap function
>>> + *   only when no vmapping exists, and only unmaps it once. Protection against
>>> + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
>> Who is talking the lock? The caller of the dma_buf_vmap/vunmap() functions,
>> the functions itself or the callback inside the exporter?
> That's the part I didn't change at all here, just re-laid out the line
> breaking. I only removed the outdated kmap section here.

I just wanted to point out that this still isn't described here very very.


> Should I do another patch and remove this one sentence here (it's kinda
> pointless and generally we don't muse about implementation details that
> callers don't care about)?

Na, works like this for me.

> I did try and do a cursory review of the dma-buf docs, but this is kinda
> not meant as an all-out revamp. Just a few things I've noticed while
> reviewing Thomas' vmap_local stuff.


Fell free to add an Acked-by: Christian König <christian.koenig@amd.com> 
to the series.

Christian.

> -Daniel
>
>> Christian.
>>
>>>     *
>>>     * - For full compatibility on the importer side with existing userspace
>>>     *   interfaces, which might already support mmap'ing buffers. This is needed in
>>> @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>>>     * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
>>>     * it guaranteed to be coherent with other DMA access.
>>>     *
>>> + * This function will also wait for any DMA transactions tracked through
>>> + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
>>> + * synchronization this function will only ensure cache coherency, callers must
>>> + * ensure synchronization with such DMA transactions on their own.
>>> + *
>>>     * Can return negative error values, returns 0 on success.
>>>     */
>>>    int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>>> @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
>>>     * This call may fail due to lack of virtual mapping address space.
>>>     * These calls are optional in drivers. The intended use for them
>>>     * is for mapping objects linear in kernel space for high use objects.
>>> - * Please attempt to use kmap/kunmap before thinking about these interfaces.
>>> + *
>>> + * To ensure coherency users must call dma_buf_begin_cpu_access() and
>>> + * dma_buf_end_cpu_access() around any cpu access performed through this
>>> + * mapping.
>>>     *
>>>     * Returns 0 on success, or a negative errno code otherwise.
>>>     */
>>> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
>>> index cf72699cb2bc..7eca37c8b10c 100644
>>> --- a/include/linux/dma-buf.h
>>> +++ b/include/linux/dma-buf.h
>>> @@ -183,24 +183,19 @@ struct dma_buf_ops {
>>>    	 * @begin_cpu_access:
>>>    	 *
>>>    	 * This is called from dma_buf_begin_cpu_access() and allows the
>>> -	 * exporter to ensure that the memory is actually available for cpu
>>> -	 * access - the exporter might need to allocate or swap-in and pin the
>>> -	 * backing storage. The exporter also needs to ensure that cpu access is
>>> -	 * coherent for the access direction. The direction can be used by the
>>> -	 * exporter to optimize the cache flushing, i.e. access with a different
>>> +	 * exporter to ensure that the memory is actually coherent for cpu
>>> +	 * access. The exporter also needs to ensure that cpu access is coherent
>>> +	 * for the access direction. The direction can be used by the exporter
>>> +	 * to optimize the cache flushing, i.e. access with a different
>>>    	 * direction (read instead of write) might return stale or even bogus
>>>    	 * data (e.g. when the exporter needs to copy the data to temporary
>>>    	 * storage).
>>>    	 *
>>> -	 * This callback is optional.
>>> +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
>>> +	 * command for userspace mappings established through @mmap, and also
>>> +	 * for kernel mappings established with @vmap.
>>>    	 *
>>> -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
>>> -	 * from userspace (where storage shouldn't be pinned to avoid handing
>>> -	 * de-factor mlock rights to userspace) and for the kernel-internal
>>> -	 * users of the various kmap interfaces, where the backing storage must
>>> -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
>>> -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
>>> -	 * real problem.
>>> +	 * This callback is optional.
>>>    	 *
>>>    	 * Returns:
>>>    	 *
>>> @@ -216,9 +211,7 @@ struct dma_buf_ops {
>>>    	 *
>>>    	 * This is called from dma_buf_end_cpu_access() when the importer is
>>>    	 * done accessing the CPU. The exporter can use this to flush caches and
>>> -	 * unpin any resources pinned in @begin_cpu_access.
>>> -	 * The result of any dma_buf kmap calls after end_cpu_access is
>>> -	 * undefined.
>>> +	 * undo anything else done in @begin_cpu_access.
>>>    	 *
>>>    	 * This callback is optional.
>>>    	 *


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
@ 2020-12-15 14:18       ` Christian König
  0 siblings, 0 replies; 44+ messages in thread
From: Christian König @ 2020-12-15 14:18 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	linaro-mm-sig, Thomas Zimmermann, Daniel Vetter, linux-media

Am 14.12.20 um 17:01 schrieb Daniel Vetter:
> On Mon, Dec 14, 2020 at 11:33:10AM +0100, Christian König wrote:
>> Am 11.12.20 um 16:58 schrieb Daniel Vetter:
>>> Also try to clarify a bit when dma_buf_begin/end_cpu_access should
>>> be called.
>>>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>> Cc: Sumit Semwal <sumit.semwal@linaro.org>
>>> Cc: "Christian König" <christian.koenig@amd.com>
>>> Cc: linux-media@vger.kernel.org
>>> Cc: linaro-mm-sig@lists.linaro.org
>>> ---
>>>    drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
>>>    include/linux/dma-buf.h   | 25 +++++++++----------------
>>>    2 files changed, 23 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index e63684d4cd90..a12fdffa130f 100644
>>> --- a/drivers/dma-buf/dma-buf.c
>>> +++ b/drivers/dma-buf/dma-buf.c
>>> @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
>>>     *   vmalloc space might be limited and result in vmap calls failing.
>>>     *
>>>     *   Interfaces::
>>> + *
>>>     *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
>>>     *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
>>>     *
>>>     *   The vmap call can fail if there is no vmap support in the exporter, or if
>>> - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
>>> - *   that the dma-buf layer keeps a reference count for all vmap access and
>>> - *   calls down into the exporter's vmap function only when no vmapping exists,
>>> - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
>>> - *   provided by taking the dma_buf->lock mutex.
>>> + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
>>> + *   count for all vmap access and calls down into the exporter's vmap function
>>> + *   only when no vmapping exists, and only unmaps it once. Protection against
>>> + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
>> Who is talking the lock? The caller of the dma_buf_vmap/vunmap() functions,
>> the functions itself or the callback inside the exporter?
> That's the part I didn't change at all here, just re-laid out the line
> breaking. I only removed the outdated kmap section here.

I just wanted to point out that this still isn't described here very very.


> Should I do another patch and remove this one sentence here (it's kinda
> pointless and generally we don't muse about implementation details that
> callers don't care about)?

Na, works like this for me.

> I did try and do a cursory review of the dma-buf docs, but this is kinda
> not meant as an all-out revamp. Just a few things I've noticed while
> reviewing Thomas' vmap_local stuff.


Fell free to add an Acked-by: Christian König <christian.koenig@amd.com> 
to the series.

Christian.

> -Daniel
>
>> Christian.
>>
>>>     *
>>>     * - For full compatibility on the importer side with existing userspace
>>>     *   interfaces, which might already support mmap'ing buffers. This is needed in
>>> @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>>>     * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
>>>     * it guaranteed to be coherent with other DMA access.
>>>     *
>>> + * This function will also wait for any DMA transactions tracked through
>>> + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
>>> + * synchronization this function will only ensure cache coherency, callers must
>>> + * ensure synchronization with such DMA transactions on their own.
>>> + *
>>>     * Can return negative error values, returns 0 on success.
>>>     */
>>>    int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>>> @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
>>>     * This call may fail due to lack of virtual mapping address space.
>>>     * These calls are optional in drivers. The intended use for them
>>>     * is for mapping objects linear in kernel space for high use objects.
>>> - * Please attempt to use kmap/kunmap before thinking about these interfaces.
>>> + *
>>> + * To ensure coherency users must call dma_buf_begin_cpu_access() and
>>> + * dma_buf_end_cpu_access() around any cpu access performed through this
>>> + * mapping.
>>>     *
>>>     * Returns 0 on success, or a negative errno code otherwise.
>>>     */
>>> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
>>> index cf72699cb2bc..7eca37c8b10c 100644
>>> --- a/include/linux/dma-buf.h
>>> +++ b/include/linux/dma-buf.h
>>> @@ -183,24 +183,19 @@ struct dma_buf_ops {
>>>    	 * @begin_cpu_access:
>>>    	 *
>>>    	 * This is called from dma_buf_begin_cpu_access() and allows the
>>> -	 * exporter to ensure that the memory is actually available for cpu
>>> -	 * access - the exporter might need to allocate or swap-in and pin the
>>> -	 * backing storage. The exporter also needs to ensure that cpu access is
>>> -	 * coherent for the access direction. The direction can be used by the
>>> -	 * exporter to optimize the cache flushing, i.e. access with a different
>>> +	 * exporter to ensure that the memory is actually coherent for cpu
>>> +	 * access. The exporter also needs to ensure that cpu access is coherent
>>> +	 * for the access direction. The direction can be used by the exporter
>>> +	 * to optimize the cache flushing, i.e. access with a different
>>>    	 * direction (read instead of write) might return stale or even bogus
>>>    	 * data (e.g. when the exporter needs to copy the data to temporary
>>>    	 * storage).
>>>    	 *
>>> -	 * This callback is optional.
>>> +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
>>> +	 * command for userspace mappings established through @mmap, and also
>>> +	 * for kernel mappings established with @vmap.
>>>    	 *
>>> -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
>>> -	 * from userspace (where storage shouldn't be pinned to avoid handing
>>> -	 * de-factor mlock rights to userspace) and for the kernel-internal
>>> -	 * users of the various kmap interfaces, where the backing storage must
>>> -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
>>> -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
>>> -	 * real problem.
>>> +	 * This callback is optional.
>>>    	 *
>>>    	 * Returns:
>>>    	 *
>>> @@ -216,9 +211,7 @@ struct dma_buf_ops {
>>>    	 *
>>>    	 * This is called from dma_buf_end_cpu_access() when the importer is
>>>    	 * done accessing the CPU. The exporter can use this to flush caches and
>>> -	 * unpin any resources pinned in @begin_cpu_access.
>>> -	 * The result of any dma_buf kmap calls after end_cpu_access is
>>> -	 * undefined.
>>> +	 * undo anything else done in @begin_cpu_access.
>>>    	 *
>>>    	 * This callback is optional.
>>>    	 *

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Intel-gfx] [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
@ 2020-12-15 14:18       ` Christian König
  0 siblings, 0 replies; 44+ messages in thread
From: Christian König @ 2020-12-15 14:18 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	linaro-mm-sig, Thomas Zimmermann, Daniel Vetter, Sumit Semwal,
	linux-media

Am 14.12.20 um 17:01 schrieb Daniel Vetter:
> On Mon, Dec 14, 2020 at 11:33:10AM +0100, Christian König wrote:
>> Am 11.12.20 um 16:58 schrieb Daniel Vetter:
>>> Also try to clarify a bit when dma_buf_begin/end_cpu_access should
>>> be called.
>>>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>> Cc: Sumit Semwal <sumit.semwal@linaro.org>
>>> Cc: "Christian König" <christian.koenig@amd.com>
>>> Cc: linux-media@vger.kernel.org
>>> Cc: linaro-mm-sig@lists.linaro.org
>>> ---
>>>    drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
>>>    include/linux/dma-buf.h   | 25 +++++++++----------------
>>>    2 files changed, 23 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index e63684d4cd90..a12fdffa130f 100644
>>> --- a/drivers/dma-buf/dma-buf.c
>>> +++ b/drivers/dma-buf/dma-buf.c
>>> @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
>>>     *   vmalloc space might be limited and result in vmap calls failing.
>>>     *
>>>     *   Interfaces::
>>> + *
>>>     *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
>>>     *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
>>>     *
>>>     *   The vmap call can fail if there is no vmap support in the exporter, or if
>>> - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
>>> - *   that the dma-buf layer keeps a reference count for all vmap access and
>>> - *   calls down into the exporter's vmap function only when no vmapping exists,
>>> - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
>>> - *   provided by taking the dma_buf->lock mutex.
>>> + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
>>> + *   count for all vmap access and calls down into the exporter's vmap function
>>> + *   only when no vmapping exists, and only unmaps it once. Protection against
>>> + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
>> Who is talking the lock? The caller of the dma_buf_vmap/vunmap() functions,
>> the functions itself or the callback inside the exporter?
> That's the part I didn't change at all here, just re-laid out the line
> breaking. I only removed the outdated kmap section here.

I just wanted to point out that this still isn't described here very very.


> Should I do another patch and remove this one sentence here (it's kinda
> pointless and generally we don't muse about implementation details that
> callers don't care about)?

Na, works like this for me.

> I did try and do a cursory review of the dma-buf docs, but this is kinda
> not meant as an all-out revamp. Just a few things I've noticed while
> reviewing Thomas' vmap_local stuff.


Fell free to add an Acked-by: Christian König <christian.koenig@amd.com> 
to the series.

Christian.

> -Daniel
>
>> Christian.
>>
>>>     *
>>>     * - For full compatibility on the importer side with existing userspace
>>>     *   interfaces, which might already support mmap'ing buffers. This is needed in
>>> @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>>>     * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
>>>     * it guaranteed to be coherent with other DMA access.
>>>     *
>>> + * This function will also wait for any DMA transactions tracked through
>>> + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
>>> + * synchronization this function will only ensure cache coherency, callers must
>>> + * ensure synchronization with such DMA transactions on their own.
>>> + *
>>>     * Can return negative error values, returns 0 on success.
>>>     */
>>>    int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
>>> @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
>>>     * This call may fail due to lack of virtual mapping address space.
>>>     * These calls are optional in drivers. The intended use for them
>>>     * is for mapping objects linear in kernel space for high use objects.
>>> - * Please attempt to use kmap/kunmap before thinking about these interfaces.
>>> + *
>>> + * To ensure coherency users must call dma_buf_begin_cpu_access() and
>>> + * dma_buf_end_cpu_access() around any cpu access performed through this
>>> + * mapping.
>>>     *
>>>     * Returns 0 on success, or a negative errno code otherwise.
>>>     */
>>> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
>>> index cf72699cb2bc..7eca37c8b10c 100644
>>> --- a/include/linux/dma-buf.h
>>> +++ b/include/linux/dma-buf.h
>>> @@ -183,24 +183,19 @@ struct dma_buf_ops {
>>>    	 * @begin_cpu_access:
>>>    	 *
>>>    	 * This is called from dma_buf_begin_cpu_access() and allows the
>>> -	 * exporter to ensure that the memory is actually available for cpu
>>> -	 * access - the exporter might need to allocate or swap-in and pin the
>>> -	 * backing storage. The exporter also needs to ensure that cpu access is
>>> -	 * coherent for the access direction. The direction can be used by the
>>> -	 * exporter to optimize the cache flushing, i.e. access with a different
>>> +	 * exporter to ensure that the memory is actually coherent for cpu
>>> +	 * access. The exporter also needs to ensure that cpu access is coherent
>>> +	 * for the access direction. The direction can be used by the exporter
>>> +	 * to optimize the cache flushing, i.e. access with a different
>>>    	 * direction (read instead of write) might return stale or even bogus
>>>    	 * data (e.g. when the exporter needs to copy the data to temporary
>>>    	 * storage).
>>>    	 *
>>> -	 * This callback is optional.
>>> +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
>>> +	 * command for userspace mappings established through @mmap, and also
>>> +	 * for kernel mappings established with @vmap.
>>>    	 *
>>> -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
>>> -	 * from userspace (where storage shouldn't be pinned to avoid handing
>>> -	 * de-factor mlock rights to userspace) and for the kernel-internal
>>> -	 * users of the various kmap interfaces, where the backing storage must
>>> -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
>>> -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
>>> -	 * real problem.
>>> +	 * This callback is optional.
>>>    	 *
>>>    	 * Returns:
>>>    	 *
>>> @@ -216,9 +211,7 @@ struct dma_buf_ops {
>>>    	 *
>>>    	 * This is called from dma_buf_end_cpu_access() when the importer is
>>>    	 * done accessing the CPU. The exporter can use this to flush caches and
>>> -	 * unpin any resources pinned in @begin_cpu_access.
>>> -	 * The result of any dma_buf kmap calls after end_cpu_access is
>>> -	 * undefined.
>>> +	 * undo anything else done in @begin_cpu_access.
>>>    	 *
>>>    	 * This callback is optional.
>>>    	 *

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev3)
  2020-12-11 15:58 ` Daniel Vetter
                   ` (9 preceding siblings ...)
  (?)
@ 2020-12-15 18:50 ` Patchwork
  -1 siblings, 0 replies; 44+ messages in thread
From: Patchwork @ 2020-12-15 18:50 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev3)
URL   : https://patchwork.freedesktop.org/series/84849/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
403e03068940 dma-buf: Remove kmap kerneldoc vestiges
-:115: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 84 lines checked
df3455aca7cf dma-buf: some kerneldoc formatting fixes
-:144: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 108 lines checked
214090d2fdc5 dma-buf: begin/end_cpu might lock the dma_resv lock
-:42: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 16 lines checked
c85f8a2cb7d4 dma-buf: doc polish for pin/unpin
-:96: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 63 lines checked


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev3)
  2020-12-11 15:58 ` Daniel Vetter
                   ` (10 preceding siblings ...)
  (?)
@ 2020-12-15 19:20 ` Patchwork
  -1 siblings, 0 replies; 44+ messages in thread
From: Patchwork @ 2020-12-15 19:20 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 2817 bytes --]

== Series Details ==

Series: series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev3)
URL   : https://patchwork.freedesktop.org/series/84849/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9488 -> Patchwork_19147
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/index.html

Known issues
------------

  Here are the changes found in Patchwork_19147 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@semaphore:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][1] ([fdo#109271]) +22 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/fi-bdw-5557u/igt@amdgpu/amd_basic@semaphore.html

  * igt@core_hotunplug@unbind-rebind:
    - fi-bdw-5557u:       NOTRUN -> [WARN][2] ([i915#2283])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/fi-bdw-5557u/igt@core_hotunplug@unbind-rebind.html

  * igt@vgem_basic@dmabuf-export:
    - fi-tgl-y:           [PASS][3] -> [DMESG-WARN][4] ([i915#402])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/fi-tgl-y/igt@vgem_basic@dmabuf-export.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/fi-tgl-y/igt@vgem_basic@dmabuf-export.html

  
#### Possible fixes ####

  * igt@gem_basic@create-fd-close:
    - fi-tgl-y:           [DMESG-WARN][5] ([i915#402]) -> [PASS][6] +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/fi-tgl-y/igt@gem_basic@create-fd-close.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/fi-tgl-y/igt@gem_basic@create-fd-close.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#2283]: https://gitlab.freedesktop.org/drm/intel/issues/2283
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402


Participating hosts (43 -> 39)
------------------------------

  Missing    (4): fi-ctg-p8600 fi-bsw-cyan fi-bdw-samus fi-hsw-4200u 


Build changes
-------------

  * Linux: CI_DRM_9488 -> Patchwork_19147

  CI-20190529: 20190529
  CI_DRM_9488: 610a032e0c8eff40d87d9344f92311382f4acd49 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5901: 565d911f08df697fa211dbd1faefe2fd57066f71 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19147: c85f8a2cb7d4f2eec063ad0627327543aba54d25 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

c85f8a2cb7d4 dma-buf: doc polish for pin/unpin
214090d2fdc5 dma-buf: begin/end_cpu might lock the dma_resv lock
df3455aca7cf dma-buf: some kerneldoc formatting fixes
403e03068940 dma-buf: Remove kmap kerneldoc vestiges

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/index.html

[-- Attachment #1.2: Type: text/html, Size: 3585 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev3)
  2020-12-11 15:58 ` Daniel Vetter
                   ` (11 preceding siblings ...)
  (?)
@ 2020-12-16  0:09 ` Patchwork
  -1 siblings, 0 replies; 44+ messages in thread
From: Patchwork @ 2020-12-16  0:09 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 23570 bytes --]

== Series Details ==

Series: series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev3)
URL   : https://patchwork.freedesktop.org/series/84849/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9488_full -> Patchwork_19147_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_19147_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_19147_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_19147_full:

### IGT changes ###

#### Warnings ####

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-kbl:          [INCOMPLETE][1] ([i915#794]) -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-kbl3/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-kbl7/igt@kms_vblank@pipe-c-ts-continuation-suspend.html

  
Known issues
------------

  Here are the changes found in Patchwork_19147_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_persistence@legacy-engines-hang:
    - shard-hsw:          NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#1099])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-hsw6/igt@gem_ctx_persistence@legacy-engines-hang.html

  * igt@gem_exec_reloc@basic-many-active@vcs1:
    - shard-iclb:         NOTRUN -> [FAIL][4] ([i915#2389])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb4/igt@gem_exec_reloc@basic-many-active@vcs1.html

  * igt@gem_userptr_blits@readonly-mmap-unsync@wb:
    - shard-iclb:         NOTRUN -> [SKIP][5] ([i915#1704]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb5/igt@gem_userptr_blits@readonly-mmap-unsync@wb.html

  * igt@gen7_exec_parse@basic-allocation:
    - shard-glk:          NOTRUN -> [SKIP][6] ([fdo#109271]) +15 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-glk7/igt@gen7_exec_parse@basic-allocation.html
    - shard-iclb:         NOTRUN -> [SKIP][7] ([fdo#109289])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb5/igt@gen7_exec_parse@basic-allocation.html

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-tglb:         [PASS][8] -> [INCOMPLETE][9] ([i915#2411] / [i915#456] / [i915#750])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-tglb6/igt@i915_pm_rpm@system-suspend-execbuf.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-tglb8/igt@i915_pm_rpm@system-suspend-execbuf.html

  * igt@kms_big_fb@yf-tiled-8bpp-rotate-0:
    - shard-iclb:         NOTRUN -> [SKIP][10] ([fdo#110723])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb5/igt@kms_big_fb@yf-tiled-8bpp-rotate-0.html

  * igt@kms_ccs@pipe-d-ccs-on-another-bo:
    - shard-iclb:         NOTRUN -> [SKIP][11] ([fdo#109278]) +4 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb5/igt@kms_ccs@pipe-d-ccs-on-another-bo.html

  * igt@kms_chamelium@dp-crc-single:
    - shard-iclb:         NOTRUN -> [SKIP][12] ([fdo#109284] / [fdo#111827]) +2 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb5/igt@kms_chamelium@dp-crc-single.html

  * igt@kms_chamelium@hdmi-hpd:
    - shard-skl:          NOTRUN -> [SKIP][13] ([fdo#109271] / [fdo#111827])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl6/igt@kms_chamelium@hdmi-hpd.html

  * igt@kms_chamelium@hdmi-hpd-after-suspend:
    - shard-hsw:          NOTRUN -> [SKIP][14] ([fdo#109271] / [fdo#111827]) +4 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-hsw6/igt@kms_chamelium@hdmi-hpd-after-suspend.html

  * igt@kms_color@pipe-a-ctm-0-5:
    - shard-skl:          [PASS][15] -> [DMESG-WARN][16] ([i915#1982])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-skl3/igt@kms_color@pipe-a-ctm-0-5.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl7/igt@kms_color@pipe-a-ctm-0-5.html

  * igt@kms_color_chamelium@pipe-d-ctm-green-to-red:
    - shard-glk:          NOTRUN -> [SKIP][17] ([fdo#109271] / [fdo#111827]) +3 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-glk7/igt@kms_color_chamelium@pipe-d-ctm-green-to-red.html
    - shard-iclb:         NOTRUN -> [SKIP][18] ([fdo#109278] / [fdo#109284] / [fdo#111827])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb5/igt@kms_color_chamelium@pipe-d-ctm-green-to-red.html

  * igt@kms_content_protection@atomic-dpms:
    - shard-iclb:         NOTRUN -> [SKIP][19] ([fdo#109300] / [fdo#111066])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb5/igt@kms_content_protection@atomic-dpms.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x128-onscreen:
    - shard-skl:          [PASS][20] -> [FAIL][21] ([i915#54]) +1 similar issue
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-skl8/igt@kms_cursor_crc@pipe-a-cursor-128x128-onscreen.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl1/igt@kms_cursor_crc@pipe-a-cursor-128x128-onscreen.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          [PASS][22] -> [FAIL][23] ([i915#2346])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-skl8/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_flip@flip-vs-expired-vblank@a-edp1:
    - shard-tglb:         [PASS][24] -> [FAIL][25] ([i915#2598]) +1 similar issue
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-tglb6/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-tglb8/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@b-hdmi-a1:
    - shard-hsw:          [PASS][26] -> [INCOMPLETE][27] ([i915#2055] / [i915#2295])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-hsw2/igt@kms_flip@flip-vs-suspend-interruptible@b-hdmi-a1.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-hsw4/igt@kms_flip@flip-vs-suspend-interruptible@b-hdmi-a1.html

  * igt@kms_flip@wf_vblank-ts-check-interruptible@a-hdmi-a1:
    - shard-glk:          [PASS][28] -> [FAIL][29] ([i915#2122])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-glk2/igt@kms_flip@wf_vblank-ts-check-interruptible@a-hdmi-a1.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-glk3/igt@kms_flip@wf_vblank-ts-check-interruptible@a-hdmi-a1.html

  * igt@kms_frontbuffer_tracking@psr-2p-primscrn-pri-shrfb-draw-mmap-wc:
    - shard-iclb:         NOTRUN -> [SKIP][30] ([fdo#109280]) +2 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb5/igt@kms_frontbuffer_tracking@psr-2p-primscrn-pri-shrfb-draw-mmap-wc.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [PASS][31] -> [FAIL][32] ([i915#1188])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-skl9/igt@kms_hdr@bpc-switch-dpms.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl2/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d:
    - shard-skl:          NOTRUN -> [SKIP][33] ([fdo#109271] / [i915#533])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl6/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [PASS][34] -> [FAIL][35] ([fdo#108145] / [i915#265])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-transparent-fb:
    - shard-glk:          NOTRUN -> [FAIL][36] ([i915#265])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-glk7/igt@kms_plane_alpha_blend@pipe-b-alpha-transparent-fb.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [PASS][37] -> [SKIP][38] ([fdo#109642] / [fdo#111068])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb7/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [PASS][39] -> [SKIP][40] ([fdo#109441]) +3 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-iclb2/igt@kms_psr@psr2_cursor_render.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb7/igt@kms_psr@psr2_cursor_render.html

  * igt@nouveau_crc@pipe-c-ctx-flip-skip-current-frame:
    - shard-hsw:          NOTRUN -> [SKIP][41] ([fdo#109271]) +52 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-hsw6/igt@nouveau_crc@pipe-c-ctx-flip-skip-current-frame.html

  * igt@nouveau_crc@pipe-d-ctx-flip-skip-current-frame:
    - shard-iclb:         NOTRUN -> [SKIP][42] ([fdo#109278] / [i915#2530])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb5/igt@nouveau_crc@pipe-d-ctx-flip-skip-current-frame.html

  * igt@prime_nv_pcopy@test1_micro:
    - shard-skl:          NOTRUN -> [SKIP][43] ([fdo#109271]) +10 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl6/igt@prime_nv_pcopy@test1_micro.html

  * igt@prime_nv_test@i915_nv_sharing:
    - shard-iclb:         NOTRUN -> [SKIP][44] ([fdo#109291])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb5/igt@prime_nv_test@i915_nv_sharing.html

  
#### Possible fixes ####

  * igt@gem_exec_reloc@basic-many-active@rcs0:
    - shard-glk:          [FAIL][45] ([i915#2389]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-glk1/igt@gem_exec_reloc@basic-many-active@rcs0.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-glk7/igt@gem_exec_reloc@basic-many-active@rcs0.html

  * {igt@gem_exec_schedule@u-fairslice@rcs0}:
    - shard-glk:          [DMESG-WARN][47] ([i915#1610]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-glk1/igt@gem_exec_schedule@u-fairslice@rcs0.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-glk7/igt@gem_exec_schedule@u-fairslice@rcs0.html
    - shard-iclb:         [DMESG-WARN][49] -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-iclb7/igt@gem_exec_schedule@u-fairslice@rcs0.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb5/igt@gem_exec_schedule@u-fairslice@rcs0.html

  * {igt@gem_vm_create@destroy-race}:
    - shard-tglb:         [TIMEOUT][51] ([i915#2795]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-tglb6/igt@gem_vm_create@destroy-race.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-tglb8/igt@gem_vm_create@destroy-race.html

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-tglb:         [FAIL][53] ([i915#2692]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-tglb2/igt@i915_pm_rc6_residency@rc6-idle.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-tglb7/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@kms_async_flips@alternate-sync-async-flip:
    - shard-skl:          [FAIL][55] ([i915#2521]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-skl6/igt@kms_async_flips@alternate-sync-async-flip.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl3/igt@kms_async_flips@alternate-sync-async-flip.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x128-sliding:
    - shard-skl:          [FAIL][57] ([i915#54]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-skl4/igt@kms_cursor_crc@pipe-a-cursor-128x128-sliding.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl10/igt@kms_cursor_crc@pipe-a-cursor-128x128-sliding.html

  * igt@kms_flip@2x-flip-vs-absolute-wf_vblank@ab-hdmi-a1-hdmi-a2:
    - shard-glk:          [FAIL][59] ([i915#2122]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-glk8/igt@kms_flip@2x-flip-vs-absolute-wf_vblank@ab-hdmi-a1-hdmi-a2.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-glk3/igt@kms_flip@2x-flip-vs-absolute-wf_vblank@ab-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@2x-flip-vs-expired-vblank@bc-hdmi-a1-hdmi-a2:
    - shard-glk:          [FAIL][61] ([i915#79]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-glk2/igt@kms_flip@2x-flip-vs-expired-vblank@bc-hdmi-a1-hdmi-a2.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-glk3/igt@kms_flip@2x-flip-vs-expired-vblank@bc-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@flip-vs-suspend-interruptible@c-edp1:
    - shard-skl:          [INCOMPLETE][63] ([i915#198] / [i915#2295]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-skl9/igt@kms_flip@flip-vs-suspend-interruptible@c-edp1.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl6/igt@kms_flip@flip-vs-suspend-interruptible@c-edp1.html

  * igt@kms_hdr@bpc-switch:
    - shard-skl:          [FAIL][65] ([i915#1188]) -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-skl6/igt@kms_hdr@bpc-switch.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl3/igt@kms_hdr@bpc-switch.html

  * igt@kms_psr@psr2_cursor_blt:
    - shard-iclb:         [SKIP][67] ([fdo#109441]) -> [PASS][68] +1 similar issue
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-iclb5/igt@kms_psr@psr2_cursor_blt.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html

  * igt@perf@blocking:
    - shard-skl:          [FAIL][69] ([i915#1542]) -> [PASS][70] +1 similar issue
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-skl6/igt@perf@blocking.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl3/igt@perf@blocking.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-iclb:         [SKIP][71] ([i915#588]) -> [SKIP][72] ([i915#658])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb7/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-skl:          [FAIL][73] ([i915#454]) -> [INCOMPLETE][74] ([i915#198])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-skl10/igt@i915_pm_dc@dc6-psr.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl4/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rc6_residency@rc6-fence:
    - shard-iclb:         [WARN][75] ([i915#2684]) -> [WARN][76] ([i915#1804] / [i915#2684])
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-iclb2/igt@i915_pm_rc6_residency@rc6-fence.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb7/igt@i915_pm_rc6_residency@rc6-fence.html

  * igt@kms_async_flips@test-time-stamp:
    - shard-tglb:         [FAIL][77] ([i915#2574]) -> [FAIL][78] ([i915#2597])
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-tglb2/igt@kms_async_flips@test-time-stamp.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-tglb7/igt@kms_async_flips@test-time-stamp.html

  * igt@runner@aborted:
    - shard-hsw:          [FAIL][79] ([i915#2295]) -> [FAIL][80] ([i915#2295] / [i915#483])
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-hsw4/igt@runner@aborted.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-hsw1/igt@runner@aborted.html
    - shard-kbl:          [FAIL][81] ([i915#2295] / [i915#2722] / [i915#483]) -> ([FAIL][82], [FAIL][83]) ([i915#1814] / [i915#2295] / [i915#2722] / [i915#602])
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-kbl1/igt@runner@aborted.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-kbl7/igt@runner@aborted.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-kbl7/igt@runner@aborted.html
    - shard-iclb:         ([FAIL][84], [FAIL][85], [FAIL][86]) ([i915#1814] / [i915#2295] / [i915#2426] / [i915#2722] / [i915#2724] / [i915#483]) -> ([FAIL][87], [FAIL][88]) ([i915#1814] / [i915#2295] / [i915#2722] / [i915#2724])
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-iclb5/igt@runner@aborted.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-iclb7/igt@runner@aborted.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-iclb4/igt@runner@aborted.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb3/igt@runner@aborted.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-iclb2/igt@runner@aborted.html
    - shard-glk:          ([FAIL][89], [FAIL][90], [FAIL][91]) ([i915#1814] / [i915#2295] / [i915#2426] / [i915#2722] / [k.org#202321]) -> ([FAIL][92], [FAIL][93]) ([i915#1814] / [i915#2295] / [i915#2722] / [k.org#202321])
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-glk1/igt@runner@aborted.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-glk9/igt@runner@aborted.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-glk5/igt@runner@aborted.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-glk6/igt@runner@aborted.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-glk4/igt@runner@aborted.html
    - shard-skl:          [FAIL][94] ([i915#2295] / [i915#2722] / [i915#483]) -> ([FAIL][95], [FAIL][96]) ([i915#2029] / [i915#2295] / [i915#2722] / [i915#483])
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9488/shard-skl5/igt@runner@aborted.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl3/igt@runner@aborted.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/shard-skl5/igt@runner@aborted.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109300]: https://bugs.freedesktop.org/show_bug.cgi?id=109300
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#111066]: https://bugs.freedesktop.org/show_bug.cgi?id=111066
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1099]: https://gitlab.freedesktop.org/drm/intel/issues/1099
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#1610]: https://gitlab.freedesktop.org/drm/intel/issues/1610
  [i915#1704]: https://gitlab.freedesktop.org/drm/intel/issues/1704
  [i915#1804]: https://gitlab.freedesktop.org/drm/intel/issues/1804
  [i915#1814]: https://gitlab.freedesktop.org/drm/intel/issues/1814
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2029]: https://gitlab.freedesktop.org/drm/intel/issues/2029
  [i915#2055]: https://gitlab.freedesktop.org/drm/intel/issues/2055
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2295]: https://gitlab.freedesktop.org/drm/intel/issues/2295
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2389]: https://gitlab.freedesktop.org/drm/intel/issues/2389
  [i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
  [i915#2426]: https://gitlab.freedesktop.org/drm/intel/issues/2426
  [i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
  [i915#2530]: https://gitlab.freedesktop.org/drm/intel/issues/2530
  [i915#2574]: https://gitlab.freedesktop.org/drm/intel/issues/2574
  [i915#2597]: https://gitlab.freedesktop.org/drm/intel/issues/2597
  [i915#2598]: https://gitlab.freedesktop.org/drm/intel/issues/2598
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#2684]: https://gitlab.freedesktop.org/drm/intel/issues/2684
  [i915#2692]: https://gitlab.freedesktop.org/drm/intel/issues/2692
  [i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722
  [i915#2724]: https://gitlab.freedesktop.org/drm/intel/issues/2724
  [i915#2795]: https://gitlab.freedesktop.org/drm/intel/issues/2795
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#456]: https://gitlab.freedesktop.org/drm/intel/issues/456
  [i915#483]: https://gitlab.freedesktop.org/drm/intel/issues/483
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
  [i915#602]: https://gitlab.freedesktop.org/drm/intel/issues/602
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#750]: https://gitlab.freedesktop.org/drm/intel/issues/750
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#794]: https://gitlab.freedesktop.org/drm/intel/issues/794
  [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_9488 -> Patchwork_19147

  CI-20190529: 20190529
  CI_DRM_9488: 610a032e0c8eff40d87d9344f92311382f4acd49 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5901: 565d911f08df697fa211dbd1faefe2fd57066f71 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19147: c85f8a2cb7d4f2eec063ad0627327543aba54d25 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19147/index.html

[-- Attachment #1.2: Type: text/html, Size: 29134 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
  2020-12-15 14:18       ` Christian König
  (?)
@ 2020-12-16 10:29         ` Daniel Vetter
  -1 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-16 10:29 UTC (permalink / raw)
  To: Christian König
  Cc: Daniel Vetter, Daniel Vetter, DRI Development,
	Intel Graphics Development, Daniel Vetter, Thomas Zimmermann,
	Sumit Semwal, linux-media, linaro-mm-sig

On Tue, Dec 15, 2020 at 03:18:49PM +0100, Christian König wrote:
> Am 14.12.20 um 17:01 schrieb Daniel Vetter:
> > On Mon, Dec 14, 2020 at 11:33:10AM +0100, Christian König wrote:
> > > Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> > > > Also try to clarify a bit when dma_buf_begin/end_cpu_access should
> > > > be called.
> > > > 
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > > > Cc: "Christian König" <christian.koenig@amd.com>
> > > > Cc: linux-media@vger.kernel.org
> > > > Cc: linaro-mm-sig@lists.linaro.org
> > > > ---
> > > >    drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
> > > >    include/linux/dma-buf.h   | 25 +++++++++----------------
> > > >    2 files changed, 23 insertions(+), 22 deletions(-)
> > > > 
> > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > > > index e63684d4cd90..a12fdffa130f 100644
> > > > --- a/drivers/dma-buf/dma-buf.c
> > > > +++ b/drivers/dma-buf/dma-buf.c
> > > > @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
> > > >     *   vmalloc space might be limited and result in vmap calls failing.
> > > >     *
> > > >     *   Interfaces::
> > > > + *
> > > >     *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
> > > >     *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
> > > >     *
> > > >     *   The vmap call can fail if there is no vmap support in the exporter, or if
> > > > - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
> > > > - *   that the dma-buf layer keeps a reference count for all vmap access and
> > > > - *   calls down into the exporter's vmap function only when no vmapping exists,
> > > > - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
> > > > - *   provided by taking the dma_buf->lock mutex.
> > > > + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
> > > > + *   count for all vmap access and calls down into the exporter's vmap function
> > > > + *   only when no vmapping exists, and only unmaps it once. Protection against
> > > > + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
> > > Who is talking the lock? The caller of the dma_buf_vmap/vunmap() functions,
> > > the functions itself or the callback inside the exporter?
> > That's the part I didn't change at all here, just re-laid out the line
> > breaking. I only removed the outdated kmap section here.
> 
> I just wanted to point out that this still isn't described here very very.
> 
> 
> > Should I do another patch and remove this one sentence here (it's kinda
> > pointless and generally we don't muse about implementation details that
> > callers don't care about)?
> 
> Na, works like this for me.
> 
> > I did try and do a cursory review of the dma-buf docs, but this is kinda
> > not meant as an all-out revamp. Just a few things I've noticed while
> > reviewing Thomas' vmap_local stuff.
> 
> 
> Fell free to add an Acked-by: Christian König <christian.koenig@amd.com> to
> the series.

Thanks for taking a look, and yeah I actually want to do a review of all
the dma-buf docs but just haven't found the quiet time for that yet.

Patches pushed to drm-misc-next.
-Daniel

> 
> Christian.
> 
> > -Daniel
> > 
> > > Christian.
> > > 
> > > >     *
> > > >     * - For full compatibility on the importer side with existing userspace
> > > >     *   interfaces, which might already support mmap'ing buffers. This is needed in
> > > > @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > > >     * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
> > > >     * it guaranteed to be coherent with other DMA access.
> > > >     *
> > > > + * This function will also wait for any DMA transactions tracked through
> > > > + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
> > > > + * synchronization this function will only ensure cache coherency, callers must
> > > > + * ensure synchronization with such DMA transactions on their own.
> > > > + *
> > > >     * Can return negative error values, returns 0 on success.
> > > >     */
> > > >    int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > > > @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
> > > >     * This call may fail due to lack of virtual mapping address space.
> > > >     * These calls are optional in drivers. The intended use for them
> > > >     * is for mapping objects linear in kernel space for high use objects.
> > > > - * Please attempt to use kmap/kunmap before thinking about these interfaces.
> > > > + *
> > > > + * To ensure coherency users must call dma_buf_begin_cpu_access() and
> > > > + * dma_buf_end_cpu_access() around any cpu access performed through this
> > > > + * mapping.
> > > >     *
> > > >     * Returns 0 on success, or a negative errno code otherwise.
> > > >     */
> > > > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > > > index cf72699cb2bc..7eca37c8b10c 100644
> > > > --- a/include/linux/dma-buf.h
> > > > +++ b/include/linux/dma-buf.h
> > > > @@ -183,24 +183,19 @@ struct dma_buf_ops {
> > > >    	 * @begin_cpu_access:
> > > >    	 *
> > > >    	 * This is called from dma_buf_begin_cpu_access() and allows the
> > > > -	 * exporter to ensure that the memory is actually available for cpu
> > > > -	 * access - the exporter might need to allocate or swap-in and pin the
> > > > -	 * backing storage. The exporter also needs to ensure that cpu access is
> > > > -	 * coherent for the access direction. The direction can be used by the
> > > > -	 * exporter to optimize the cache flushing, i.e. access with a different
> > > > +	 * exporter to ensure that the memory is actually coherent for cpu
> > > > +	 * access. The exporter also needs to ensure that cpu access is coherent
> > > > +	 * for the access direction. The direction can be used by the exporter
> > > > +	 * to optimize the cache flushing, i.e. access with a different
> > > >    	 * direction (read instead of write) might return stale or even bogus
> > > >    	 * data (e.g. when the exporter needs to copy the data to temporary
> > > >    	 * storage).
> > > >    	 *
> > > > -	 * This callback is optional.
> > > > +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
> > > > +	 * command for userspace mappings established through @mmap, and also
> > > > +	 * for kernel mappings established with @vmap.
> > > >    	 *
> > > > -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
> > > > -	 * from userspace (where storage shouldn't be pinned to avoid handing
> > > > -	 * de-factor mlock rights to userspace) and for the kernel-internal
> > > > -	 * users of the various kmap interfaces, where the backing storage must
> > > > -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
> > > > -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
> > > > -	 * real problem.
> > > > +	 * This callback is optional.
> > > >    	 *
> > > >    	 * Returns:
> > > >    	 *
> > > > @@ -216,9 +211,7 @@ struct dma_buf_ops {
> > > >    	 *
> > > >    	 * This is called from dma_buf_end_cpu_access() when the importer is
> > > >    	 * done accessing the CPU. The exporter can use this to flush caches and
> > > > -	 * unpin any resources pinned in @begin_cpu_access.
> > > > -	 * The result of any dma_buf kmap calls after end_cpu_access is
> > > > -	 * undefined.
> > > > +	 * undo anything else done in @begin_cpu_access.
> > > >    	 *
> > > >    	 * This callback is optional.
> > > >    	 *
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
@ 2020-12-16 10:29         ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-16 10:29 UTC (permalink / raw)
  To: Christian König
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	linaro-mm-sig, Thomas Zimmermann, Daniel Vetter, linux-media

On Tue, Dec 15, 2020 at 03:18:49PM +0100, Christian König wrote:
> Am 14.12.20 um 17:01 schrieb Daniel Vetter:
> > On Mon, Dec 14, 2020 at 11:33:10AM +0100, Christian König wrote:
> > > Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> > > > Also try to clarify a bit when dma_buf_begin/end_cpu_access should
> > > > be called.
> > > > 
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > > > Cc: "Christian König" <christian.koenig@amd.com>
> > > > Cc: linux-media@vger.kernel.org
> > > > Cc: linaro-mm-sig@lists.linaro.org
> > > > ---
> > > >    drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
> > > >    include/linux/dma-buf.h   | 25 +++++++++----------------
> > > >    2 files changed, 23 insertions(+), 22 deletions(-)
> > > > 
> > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > > > index e63684d4cd90..a12fdffa130f 100644
> > > > --- a/drivers/dma-buf/dma-buf.c
> > > > +++ b/drivers/dma-buf/dma-buf.c
> > > > @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
> > > >     *   vmalloc space might be limited and result in vmap calls failing.
> > > >     *
> > > >     *   Interfaces::
> > > > + *
> > > >     *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
> > > >     *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
> > > >     *
> > > >     *   The vmap call can fail if there is no vmap support in the exporter, or if
> > > > - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
> > > > - *   that the dma-buf layer keeps a reference count for all vmap access and
> > > > - *   calls down into the exporter's vmap function only when no vmapping exists,
> > > > - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
> > > > - *   provided by taking the dma_buf->lock mutex.
> > > > + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
> > > > + *   count for all vmap access and calls down into the exporter's vmap function
> > > > + *   only when no vmapping exists, and only unmaps it once. Protection against
> > > > + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
> > > Who is talking the lock? The caller of the dma_buf_vmap/vunmap() functions,
> > > the functions itself or the callback inside the exporter?
> > That's the part I didn't change at all here, just re-laid out the line
> > breaking. I only removed the outdated kmap section here.
> 
> I just wanted to point out that this still isn't described here very very.
> 
> 
> > Should I do another patch and remove this one sentence here (it's kinda
> > pointless and generally we don't muse about implementation details that
> > callers don't care about)?
> 
> Na, works like this for me.
> 
> > I did try and do a cursory review of the dma-buf docs, but this is kinda
> > not meant as an all-out revamp. Just a few things I've noticed while
> > reviewing Thomas' vmap_local stuff.
> 
> 
> Fell free to add an Acked-by: Christian König <christian.koenig@amd.com> to
> the series.

Thanks for taking a look, and yeah I actually want to do a review of all
the dma-buf docs but just haven't found the quiet time for that yet.

Patches pushed to drm-misc-next.
-Daniel

> 
> Christian.
> 
> > -Daniel
> > 
> > > Christian.
> > > 
> > > >     *
> > > >     * - For full compatibility on the importer side with existing userspace
> > > >     *   interfaces, which might already support mmap'ing buffers. This is needed in
> > > > @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > > >     * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
> > > >     * it guaranteed to be coherent with other DMA access.
> > > >     *
> > > > + * This function will also wait for any DMA transactions tracked through
> > > > + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
> > > > + * synchronization this function will only ensure cache coherency, callers must
> > > > + * ensure synchronization with such DMA transactions on their own.
> > > > + *
> > > >     * Can return negative error values, returns 0 on success.
> > > >     */
> > > >    int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > > > @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
> > > >     * This call may fail due to lack of virtual mapping address space.
> > > >     * These calls are optional in drivers. The intended use for them
> > > >     * is for mapping objects linear in kernel space for high use objects.
> > > > - * Please attempt to use kmap/kunmap before thinking about these interfaces.
> > > > + *
> > > > + * To ensure coherency users must call dma_buf_begin_cpu_access() and
> > > > + * dma_buf_end_cpu_access() around any cpu access performed through this
> > > > + * mapping.
> > > >     *
> > > >     * Returns 0 on success, or a negative errno code otherwise.
> > > >     */
> > > > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > > > index cf72699cb2bc..7eca37c8b10c 100644
> > > > --- a/include/linux/dma-buf.h
> > > > +++ b/include/linux/dma-buf.h
> > > > @@ -183,24 +183,19 @@ struct dma_buf_ops {
> > > >    	 * @begin_cpu_access:
> > > >    	 *
> > > >    	 * This is called from dma_buf_begin_cpu_access() and allows the
> > > > -	 * exporter to ensure that the memory is actually available for cpu
> > > > -	 * access - the exporter might need to allocate or swap-in and pin the
> > > > -	 * backing storage. The exporter also needs to ensure that cpu access is
> > > > -	 * coherent for the access direction. The direction can be used by the
> > > > -	 * exporter to optimize the cache flushing, i.e. access with a different
> > > > +	 * exporter to ensure that the memory is actually coherent for cpu
> > > > +	 * access. The exporter also needs to ensure that cpu access is coherent
> > > > +	 * for the access direction. The direction can be used by the exporter
> > > > +	 * to optimize the cache flushing, i.e. access with a different
> > > >    	 * direction (read instead of write) might return stale or even bogus
> > > >    	 * data (e.g. when the exporter needs to copy the data to temporary
> > > >    	 * storage).
> > > >    	 *
> > > > -	 * This callback is optional.
> > > > +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
> > > > +	 * command for userspace mappings established through @mmap, and also
> > > > +	 * for kernel mappings established with @vmap.
> > > >    	 *
> > > > -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
> > > > -	 * from userspace (where storage shouldn't be pinned to avoid handing
> > > > -	 * de-factor mlock rights to userspace) and for the kernel-internal
> > > > -	 * users of the various kmap interfaces, where the backing storage must
> > > > -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
> > > > -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
> > > > -	 * real problem.
> > > > +	 * This callback is optional.
> > > >    	 *
> > > >    	 * Returns:
> > > >    	 *
> > > > @@ -216,9 +211,7 @@ struct dma_buf_ops {
> > > >    	 *
> > > >    	 * This is called from dma_buf_end_cpu_access() when the importer is
> > > >    	 * done accessing the CPU. The exporter can use this to flush caches and
> > > > -	 * unpin any resources pinned in @begin_cpu_access.
> > > > -	 * The result of any dma_buf kmap calls after end_cpu_access is
> > > > -	 * undefined.
> > > > +	 * undo anything else done in @begin_cpu_access.
> > > >    	 *
> > > >    	 * This callback is optional.
> > > >    	 *
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Intel-gfx] [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges
@ 2020-12-16 10:29         ` Daniel Vetter
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel Vetter @ 2020-12-16 10:29 UTC (permalink / raw)
  To: Christian König
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	linaro-mm-sig, Thomas Zimmermann, Daniel Vetter, Sumit Semwal,
	linux-media

On Tue, Dec 15, 2020 at 03:18:49PM +0100, Christian König wrote:
> Am 14.12.20 um 17:01 schrieb Daniel Vetter:
> > On Mon, Dec 14, 2020 at 11:33:10AM +0100, Christian König wrote:
> > > Am 11.12.20 um 16:58 schrieb Daniel Vetter:
> > > > Also try to clarify a bit when dma_buf_begin/end_cpu_access should
> > > > be called.
> > > > 
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > > > Cc: "Christian König" <christian.koenig@amd.com>
> > > > Cc: linux-media@vger.kernel.org
> > > > Cc: linaro-mm-sig@lists.linaro.org
> > > > ---
> > > >    drivers/dma-buf/dma-buf.c | 20 ++++++++++++++------
> > > >    include/linux/dma-buf.h   | 25 +++++++++----------------
> > > >    2 files changed, 23 insertions(+), 22 deletions(-)
> > > > 
> > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > > > index e63684d4cd90..a12fdffa130f 100644
> > > > --- a/drivers/dma-buf/dma-buf.c
> > > > +++ b/drivers/dma-buf/dma-buf.c
> > > > @@ -1001,15 +1001,15 @@ EXPORT_SYMBOL_GPL(dma_buf_move_notify);
> > > >     *   vmalloc space might be limited and result in vmap calls failing.
> > > >     *
> > > >     *   Interfaces::
> > > > + *
> > > >     *      void \*dma_buf_vmap(struct dma_buf \*dmabuf)
> > > >     *      void dma_buf_vunmap(struct dma_buf \*dmabuf, void \*vaddr)
> > > >     *
> > > >     *   The vmap call can fail if there is no vmap support in the exporter, or if
> > > > - *   it runs out of vmalloc space. Fallback to kmap should be implemented. Note
> > > > - *   that the dma-buf layer keeps a reference count for all vmap access and
> > > > - *   calls down into the exporter's vmap function only when no vmapping exists,
> > > > - *   and only unmaps it once. Protection against concurrent vmap/vunmap calls is
> > > > - *   provided by taking the dma_buf->lock mutex.
> > > > + *   it runs out of vmalloc space. Note that the dma-buf layer keeps a reference
> > > > + *   count for all vmap access and calls down into the exporter's vmap function
> > > > + *   only when no vmapping exists, and only unmaps it once. Protection against
> > > > + *   concurrent vmap/vunmap calls is provided by taking the &dma_buf.lock mutex.
> > > Who is talking the lock? The caller of the dma_buf_vmap/vunmap() functions,
> > > the functions itself or the callback inside the exporter?
> > That's the part I didn't change at all here, just re-laid out the line
> > breaking. I only removed the outdated kmap section here.
> 
> I just wanted to point out that this still isn't described here very very.
> 
> 
> > Should I do another patch and remove this one sentence here (it's kinda
> > pointless and generally we don't muse about implementation details that
> > callers don't care about)?
> 
> Na, works like this for me.
> 
> > I did try and do a cursory review of the dma-buf docs, but this is kinda
> > not meant as an all-out revamp. Just a few things I've noticed while
> > reviewing Thomas' vmap_local stuff.
> 
> 
> Fell free to add an Acked-by: Christian König <christian.koenig@amd.com> to
> the series.

Thanks for taking a look, and yeah I actually want to do a review of all
the dma-buf docs but just haven't found the quiet time for that yet.

Patches pushed to drm-misc-next.
-Daniel

> 
> Christian.
> 
> > -Daniel
> > 
> > > Christian.
> > > 
> > > >     *
> > > >     * - For full compatibility on the importer side with existing userspace
> > > >     *   interfaces, which might already support mmap'ing buffers. This is needed in
> > > > @@ -1098,6 +1098,11 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > > >     * dma_buf_end_cpu_access(). Only when cpu access is braketed by both calls is
> > > >     * it guaranteed to be coherent with other DMA access.
> > > >     *
> > > > + * This function will also wait for any DMA transactions tracked through
> > > > + * implicit synchronization in &dma_buf.resv. For DMA transactions with explicit
> > > > + * synchronization this function will only ensure cache coherency, callers must
> > > > + * ensure synchronization with such DMA transactions on their own.
> > > > + *
> > > >     * Can return negative error values, returns 0 on success.
> > > >     */
> > > >    int dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
> > > > @@ -1199,7 +1204,10 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
> > > >     * This call may fail due to lack of virtual mapping address space.
> > > >     * These calls are optional in drivers. The intended use for them
> > > >     * is for mapping objects linear in kernel space for high use objects.
> > > > - * Please attempt to use kmap/kunmap before thinking about these interfaces.
> > > > + *
> > > > + * To ensure coherency users must call dma_buf_begin_cpu_access() and
> > > > + * dma_buf_end_cpu_access() around any cpu access performed through this
> > > > + * mapping.
> > > >     *
> > > >     * Returns 0 on success, or a negative errno code otherwise.
> > > >     */
> > > > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > > > index cf72699cb2bc..7eca37c8b10c 100644
> > > > --- a/include/linux/dma-buf.h
> > > > +++ b/include/linux/dma-buf.h
> > > > @@ -183,24 +183,19 @@ struct dma_buf_ops {
> > > >    	 * @begin_cpu_access:
> > > >    	 *
> > > >    	 * This is called from dma_buf_begin_cpu_access() and allows the
> > > > -	 * exporter to ensure that the memory is actually available for cpu
> > > > -	 * access - the exporter might need to allocate or swap-in and pin the
> > > > -	 * backing storage. The exporter also needs to ensure that cpu access is
> > > > -	 * coherent for the access direction. The direction can be used by the
> > > > -	 * exporter to optimize the cache flushing, i.e. access with a different
> > > > +	 * exporter to ensure that the memory is actually coherent for cpu
> > > > +	 * access. The exporter also needs to ensure that cpu access is coherent
> > > > +	 * for the access direction. The direction can be used by the exporter
> > > > +	 * to optimize the cache flushing, i.e. access with a different
> > > >    	 * direction (read instead of write) might return stale or even bogus
> > > >    	 * data (e.g. when the exporter needs to copy the data to temporary
> > > >    	 * storage).
> > > >    	 *
> > > > -	 * This callback is optional.
> > > > +	 * Note that this is both called through the DMA_BUF_IOCTL_SYNC IOCTL
> > > > +	 * command for userspace mappings established through @mmap, and also
> > > > +	 * for kernel mappings established with @vmap.
> > > >    	 *
> > > > -	 * FIXME: This is both called through the DMA_BUF_IOCTL_SYNC command
> > > > -	 * from userspace (where storage shouldn't be pinned to avoid handing
> > > > -	 * de-factor mlock rights to userspace) and for the kernel-internal
> > > > -	 * users of the various kmap interfaces, where the backing storage must
> > > > -	 * be pinned to guarantee that the atomic kmap calls can succeed. Since
> > > > -	 * there's no in-kernel users of the kmap interfaces yet this isn't a
> > > > -	 * real problem.
> > > > +	 * This callback is optional.
> > > >    	 *
> > > >    	 * Returns:
> > > >    	 *
> > > > @@ -216,9 +211,7 @@ struct dma_buf_ops {
> > > >    	 *
> > > >    	 * This is called from dma_buf_end_cpu_access() when the importer is
> > > >    	 * done accessing the CPU. The exporter can use this to flush caches and
> > > > -	 * unpin any resources pinned in @begin_cpu_access.
> > > > -	 * The result of any dma_buf kmap calls after end_cpu_access is
> > > > -	 * undefined.
> > > > +	 * undo anything else done in @begin_cpu_access.
> > > >    	 *
> > > >    	 * This callback is optional.
> > > >    	 *
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2020-12-16 10:30 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-11 15:58 [PATCH 1/4] dma-buf: Remove kmap kerneldoc vestiges Daniel Vetter
2020-12-11 15:58 ` [Intel-gfx] " Daniel Vetter
2020-12-11 15:58 ` Daniel Vetter
2020-12-11 15:58 ` [PATCH 2/4] dma-buf: some kerneldoc formatting fixes Daniel Vetter
2020-12-11 15:58   ` [Intel-gfx] " Daniel Vetter
2020-12-11 15:58   ` Daniel Vetter
2020-12-11 15:58 ` [PATCH 3/4] dma-buf: begin/end_cpu might lock the dma_resv lock Daniel Vetter
2020-12-11 15:58   ` [Intel-gfx] " Daniel Vetter
2020-12-11 15:58   ` Daniel Vetter
2020-12-11 22:31   ` kernel test robot
2020-12-11 22:31     ` kernel test robot
2020-12-11 22:31     ` [Intel-gfx] " kernel test robot
2020-12-11 22:31     ` kernel test robot
2020-12-14 10:33   ` Christian König
2020-12-14 10:33     ` [Intel-gfx] " Christian König
2020-12-14 10:33     ` Christian König
2020-12-14 17:16   ` [PATCH] " Daniel Vetter
2020-12-14 17:16     ` [Intel-gfx] " Daniel Vetter
2020-12-14 17:16     ` Daniel Vetter
2020-12-15 14:16     ` Christian König
2020-12-15 14:16       ` [Intel-gfx] " Christian König
2020-12-15 14:16       ` Christian König
2020-12-11 15:58 ` [PATCH 4/4] dma-buf: doc polish for pin/unpin Daniel Vetter
2020-12-11 15:58   ` [Intel-gfx] " Daniel Vetter
2020-12-11 15:58   ` Daniel Vetter
2020-12-11 16:21 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges Patchwork
2020-12-14 10:33 ` [PATCH 1/4] " Christian König
2020-12-14 10:33   ` [Intel-gfx] " Christian König
2020-12-14 10:33   ` Christian König
2020-12-14 16:01   ` Daniel Vetter
2020-12-14 16:01     ` [Intel-gfx] " Daniel Vetter
2020-12-14 16:01     ` Daniel Vetter
2020-12-15 14:18     ` Christian König
2020-12-15 14:18       ` [Intel-gfx] " Christian König
2020-12-15 14:18       ` Christian König
2020-12-16 10:29       ` Daniel Vetter
2020-12-16 10:29         ` [Intel-gfx] " Daniel Vetter
2020-12-16 10:29         ` Daniel Vetter
2020-12-14 18:02 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev2) Patchwork
2020-12-14 18:06 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2020-12-14 18:33 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-12-15 18:50 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] dma-buf: Remove kmap kerneldoc vestiges (rev3) Patchwork
2020-12-15 19:20 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-12-16  0:09 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.