diff mbox series

[v4,01/13] dma-buf: Add vmap_local and vnumap_local operations

Message ID 20210108094340.15290-2-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm: Support short-term vmap via vmap_local | expand

Commit Message

Thomas Zimmermann Jan. 8, 2021, 9:43 a.m. UTC
The existing dma-buf calls dma_buf_vmap() and dma_buf_vunmap() are
allowed to pin the buffer or acquire the buffer's reservation object
lock.

This is a problem for callers that only require a short-term mapping
of the buffer without the pinning, or callers that have special locking
requirements. These may suffer from unnecessary overhead or interfere
with regular pin operations.

The new interfaces dma_buf_vmap_local(), dma_buf_vunmapo_local(), and
their rsp callbacks in struct dma_buf_ops provide an alternative without
pinning or reservation locking. Callers are responsible for these
operations.

v4:
	* update documentation (Daniel)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/dma-buf.c | 81 +++++++++++++++++++++++++++++++++++++++
 include/linux/dma-buf.h   | 34 ++++++++++++++++
 2 files changed, 115 insertions(+)

Comments

Michael J. Ruhl Jan. 8, 2021, 4:12 p.m. UTC | #1
>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>Thomas Zimmermann
>Sent: Friday, January 8, 2021 4:43 AM
>To: sumit.semwal@linaro.org; christian.koenig@amd.com;
>airlied@redhat.com; daniel@ffwll.ch; maarten.lankhorst@linux.intel.com;
>mripard@kernel.org; kraxel@redhat.com; hdegoede@redhat.com;
>sean@poorly.run; eric@anholt.net; sam@ravnborg.org
>Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; dri-devel@lists.freedesktop.org;
>virtualization@lists.linux-foundation.org; linaro-mm-sig@lists.linaro.org;
>Thomas Zimmermann <tzimmermann@suse.de>; linux-
>media@vger.kernel.org
>Subject: [PATCH v4 01/13] dma-buf: Add vmap_local and vnumap_local
>operations
>
>The existing dma-buf calls dma_buf_vmap() and dma_buf_vunmap() are
>allowed to pin the buffer or acquire the buffer's reservation object
>lock.
>
>This is a problem for callers that only require a short-term mapping
>of the buffer without the pinning, or callers that have special locking
>requirements. These may suffer from unnecessary overhead or interfere
>with regular pin operations.
>
>The new interfaces dma_buf_vmap_local(), dma_buf_vunmapo_local(), and
>their rsp callbacks in struct dma_buf_ops provide an alternative without
>pinning or reservation locking. Callers are responsible for these
>operations.
>
>v4:
>	* update documentation (Daniel)
>
>Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>---
> drivers/dma-buf/dma-buf.c | 81
>+++++++++++++++++++++++++++++++++++++++
> include/linux/dma-buf.h   | 34 ++++++++++++++++
> 2 files changed, 115 insertions(+)
>
>diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>index b8465243eca2..01f9c74d97fa 100644
>--- a/drivers/dma-buf/dma-buf.c
>+++ b/drivers/dma-buf/dma-buf.c
>@@ -1295,6 +1295,87 @@ void dma_buf_vunmap(struct dma_buf *dmabuf,
>struct dma_buf_map *map)
> }
> EXPORT_SYMBOL_GPL(dma_buf_vunmap);
>
>+/**
>+ * dma_buf_vmap_local - Create virtual mapping for the buffer object into
>kernel
>+ * address space.
>+ * @dmabuf:	[in]	buffer to vmap
>+ * @map:	[out]	returns the vmap pointer
>+ *
>+ * Unlike dma_buf_vmap() this is a short term mapping and will not pin
>+ * the buffer. The struct dma_resv for the @dmabuf must be locked until
>+ * dma_buf_vunmap_local() is called.
>+ *
>+ * Returns:
>+ * 0 on success, or a negative errno code otherwise.
>+ */
>+int dma_buf_vmap_local(struct dma_buf *dmabuf, struct dma_buf_map
>*map)
>+{
>+	struct dma_buf_map ptr;
>+	int ret = 0;
>+
>+	dma_buf_map_clear(map);
>+
>+	if (WARN_ON(!dmabuf))
>+		return -EINVAL;
>+
>+	dma_resv_assert_held(dmabuf->resv);
>+
>+	if (!dmabuf->ops->vmap_local)
>+		return -EINVAL;

You are clearing the map, and then doing the above checks.

Is it ok to change the map info and then exit on error?

Mike

>+	mutex_lock(&dmabuf->lock);
>+	if (dmabuf->vmapping_counter) {
>+		dmabuf->vmapping_counter++;
>+		BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr));
>+		*map = dmabuf->vmap_ptr;
>+		goto out_unlock;
>+	}
>+
>+	BUG_ON(dma_buf_map_is_set(&dmabuf->vmap_ptr));
>+
>+	ret = dmabuf->ops->vmap_local(dmabuf, &ptr);
>+	if (WARN_ON_ONCE(ret))
>+		goto out_unlock;
>+
>+	dmabuf->vmap_ptr = ptr;
>+	dmabuf->vmapping_counter = 1;
>+
>+	*map = dmabuf->vmap_ptr;
>+
>+out_unlock:
>+	mutex_unlock(&dmabuf->lock);
>+	return ret;
>+}
>+EXPORT_SYMBOL_GPL(dma_buf_vmap_local);
>+
>+/**
>+ * dma_buf_vunmap_local - Unmap a vmap obtained by
>dma_buf_vmap_local.
>+ * @dmabuf:	[in]	buffer to vunmap
>+ * @map:	[in]	vmap pointer to vunmap
>+ *
>+ * Release a mapping established with dma_buf_vmap_local().
>+ */
>+void dma_buf_vunmap_local(struct dma_buf *dmabuf, struct
>dma_buf_map *map)
>+{
>+	if (WARN_ON(!dmabuf))
>+		return;
>+
>+	dma_resv_assert_held(dmabuf->resv);
>+
>+	BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr));
>+	BUG_ON(dmabuf->vmapping_counter == 0);
>+	BUG_ON(!dma_buf_map_is_equal(&dmabuf->vmap_ptr, map));
>+
>+	mutex_lock(&dmabuf->lock);
>+	if (--dmabuf->vmapping_counter == 0) {
>+		if (dmabuf->ops->vunmap_local)
>+			dmabuf->ops->vunmap_local(dmabuf, map);
>+		dma_buf_map_clear(&dmabuf->vmap_ptr);
>+	}
>+	mutex_unlock(&dmabuf->lock);
>+}
>+EXPORT_SYMBOL_GPL(dma_buf_vunmap_local);
>+
> #ifdef CONFIG_DEBUG_FS
> static int dma_buf_debug_show(struct seq_file *s, void *unused)
> {
>diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
>index 628681bf6c99..aeed754b5467 100644
>--- a/include/linux/dma-buf.h
>+++ b/include/linux/dma-buf.h
>@@ -264,6 +264,38 @@ struct dma_buf_ops {
>
> 	int (*vmap)(struct dma_buf *dmabuf, struct dma_buf_map *map);
> 	void (*vunmap)(struct dma_buf *dmabuf, struct dma_buf_map
>*map);
>+
>+	/**
>+	 * @vmap_local:
>+	 *
>+	 * Creates a virtual mapping for the buffer into kernel address space.
>+	 *
>+	 * This callback establishes short-term mappings for situations where
>+	 * callers only use the buffer for a bounded amount of time; such as
>+	 * updates to the framebuffer or reading back contained information.
>+	 * In contrast to the regular @vmap callback, vmap_local does never
>pin
>+	 * the buffer to a specific domain or acquire the buffer's reservation
>+	 * lock.
>+	 *
>+	 * This is called with the &dma_buf.resv object locked. Callers must
>hold
>+	 * the lock until after removing the mapping with @vunmap_local.
>+	 *
>+	 * This callback is optional.
>+	 *
>+	 * Returns:
>+	 *
>+	 * 0 on success or a negative error code on failure.
>+	 */
>+	int (*vmap_local)(struct dma_buf *dmabuf, struct dma_buf_map
>*map);
>+
>+	/**
>+	 * @vunmap_local:
>+	 *
>+	 * Removes a virtual mapping that was established by @vmap_local.
>+	 *
>+	 * This callback is optional.
>+	 */
>+	void (*vunmap_local)(struct dma_buf *dmabuf, struct dma_buf_map
>*map);
> };
>
> /**
>@@ -501,4 +533,6 @@ int dma_buf_mmap(struct dma_buf *, struct
>vm_area_struct *,
> 		 unsigned long);
> int dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map);
> void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map
>*map);
>+int dma_buf_vmap_local(struct dma_buf *dmabuf, struct dma_buf_map
>*map);
>+void dma_buf_vunmap_local(struct dma_buf *dmabuf, struct
>dma_buf_map *map);
> #endif /* __DMA_BUF_H__ */
>--
>2.29.2
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
Thomas Zimmermann Jan. 12, 2021, 7:45 a.m. UTC | #2
Hi

Am 08.01.21 um 17:12 schrieb Ruhl, Michael J:
>> -----Original Message-----
>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>> Thomas Zimmermann
>> Sent: Friday, January 8, 2021 4:43 AM
>> To: sumit.semwal@linaro.org; christian.koenig@amd.com;
>> airlied@redhat.com; daniel@ffwll.ch; maarten.lankhorst@linux.intel.com;
>> mripard@kernel.org; kraxel@redhat.com; hdegoede@redhat.com;
>> sean@poorly.run; eric@anholt.net; sam@ravnborg.org
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; dri-devel@lists.freedesktop.org;
>> virtualization@lists.linux-foundation.org; linaro-mm-sig@lists.linaro.org;
>> Thomas Zimmermann <tzimmermann@suse.de>; linux-
>> media@vger.kernel.org
>> Subject: [PATCH v4 01/13] dma-buf: Add vmap_local and vnumap_local
>> operations
>>
>> The existing dma-buf calls dma_buf_vmap() and dma_buf_vunmap() are
>> allowed to pin the buffer or acquire the buffer's reservation object
>> lock.
>>
>> This is a problem for callers that only require a short-term mapping
>> of the buffer without the pinning, or callers that have special locking
>> requirements. These may suffer from unnecessary overhead or interfere
>> with regular pin operations.
>>
>> The new interfaces dma_buf_vmap_local(), dma_buf_vunmapo_local(), and
>> their rsp callbacks in struct dma_buf_ops provide an alternative without
>> pinning or reservation locking. Callers are responsible for these
>> operations.
>>
>> v4:
>> 	* update documentation (Daniel)
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>> drivers/dma-buf/dma-buf.c | 81
>> +++++++++++++++++++++++++++++++++++++++
>> include/linux/dma-buf.h   | 34 ++++++++++++++++
>> 2 files changed, 115 insertions(+)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index b8465243eca2..01f9c74d97fa 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -1295,6 +1295,87 @@ void dma_buf_vunmap(struct dma_buf *dmabuf,
>> struct dma_buf_map *map)
>> }
>> EXPORT_SYMBOL_GPL(dma_buf_vunmap);
>>
>> +/**
>> + * dma_buf_vmap_local - Create virtual mapping for the buffer object into
>> kernel
>> + * address space.
>> + * @dmabuf:	[in]	buffer to vmap
>> + * @map:	[out]	returns the vmap pointer
>> + *
>> + * Unlike dma_buf_vmap() this is a short term mapping and will not pin
>> + * the buffer. The struct dma_resv for the @dmabuf must be locked until
>> + * dma_buf_vunmap_local() is called.
>> + *
>> + * Returns:
>> + * 0 on success, or a negative errno code otherwise.
>> + */
>> +int dma_buf_vmap_local(struct dma_buf *dmabuf, struct dma_buf_map
>> *map)
>> +{
>> +	struct dma_buf_map ptr;
>> +	int ret = 0;
>> +
>> +	dma_buf_map_clear(map);
>> +
>> +	if (WARN_ON(!dmabuf))
>> +		return -EINVAL;
>> +
>> +	dma_resv_assert_held(dmabuf->resv);
>> +
>> +	if (!dmabuf->ops->vmap_local)
>> +		return -EINVAL;
> 
> You are clearing the map, and then doing the above checks.
> 
> Is it ok to change the map info and then exit on error?

In vmap_local map argument returns the mapping's address. Callers are 
expected to check the return code. But I would expect a careless caller 
to not check, or check for map being cleared. Clearing it here first is 
the save thing to do.

Best regards
Thomas

> 
> Mike
> 
>> +	mutex_lock(&dmabuf->lock);
>> +	if (dmabuf->vmapping_counter) {
>> +		dmabuf->vmapping_counter++;
>> +		BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr));
>> +		*map = dmabuf->vmap_ptr;
>> +		goto out_unlock;
>> +	}
>> +
>> +	BUG_ON(dma_buf_map_is_set(&dmabuf->vmap_ptr));
>> +
>> +	ret = dmabuf->ops->vmap_local(dmabuf, &ptr);
>> +	if (WARN_ON_ONCE(ret))
>> +		goto out_unlock;
>> +
>> +	dmabuf->vmap_ptr = ptr;
>> +	dmabuf->vmapping_counter = 1;
>> +
>> +	*map = dmabuf->vmap_ptr;
>> +
>> +out_unlock:
>> +	mutex_unlock(&dmabuf->lock);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(dma_buf_vmap_local);
>> +
>> +/**
>> + * dma_buf_vunmap_local - Unmap a vmap obtained by
>> dma_buf_vmap_local.
>> + * @dmabuf:	[in]	buffer to vunmap
>> + * @map:	[in]	vmap pointer to vunmap
>> + *
>> + * Release a mapping established with dma_buf_vmap_local().
>> + */
>> +void dma_buf_vunmap_local(struct dma_buf *dmabuf, struct
>> dma_buf_map *map)
>> +{
>> +	if (WARN_ON(!dmabuf))
>> +		return;
>> +
>> +	dma_resv_assert_held(dmabuf->resv);
>> +
>> +	BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr));
>> +	BUG_ON(dmabuf->vmapping_counter == 0);
>> +	BUG_ON(!dma_buf_map_is_equal(&dmabuf->vmap_ptr, map));
>> +
>> +	mutex_lock(&dmabuf->lock);
>> +	if (--dmabuf->vmapping_counter == 0) {
>> +		if (dmabuf->ops->vunmap_local)
>> +			dmabuf->ops->vunmap_local(dmabuf, map);
>> +		dma_buf_map_clear(&dmabuf->vmap_ptr);
>> +	}
>> +	mutex_unlock(&dmabuf->lock);
>> +}
>> +EXPORT_SYMBOL_GPL(dma_buf_vunmap_local);
>> +
>> #ifdef CONFIG_DEBUG_FS
>> static int dma_buf_debug_show(struct seq_file *s, void *unused)
>> {
>> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
>> index 628681bf6c99..aeed754b5467 100644
>> --- a/include/linux/dma-buf.h
>> +++ b/include/linux/dma-buf.h
>> @@ -264,6 +264,38 @@ struct dma_buf_ops {
>>
>> 	int (*vmap)(struct dma_buf *dmabuf, struct dma_buf_map *map);
>> 	void (*vunmap)(struct dma_buf *dmabuf, struct dma_buf_map
>> *map);
>> +
>> +	/**
>> +	 * @vmap_local:
>> +	 *
>> +	 * Creates a virtual mapping for the buffer into kernel address space.
>> +	 *
>> +	 * This callback establishes short-term mappings for situations where
>> +	 * callers only use the buffer for a bounded amount of time; such as
>> +	 * updates to the framebuffer or reading back contained information.
>> +	 * In contrast to the regular @vmap callback, vmap_local does never
>> pin
>> +	 * the buffer to a specific domain or acquire the buffer's reservation
>> +	 * lock.
>> +	 *
>> +	 * This is called with the &dma_buf.resv object locked. Callers must
>> hold
>> +	 * the lock until after removing the mapping with @vunmap_local.
>> +	 *
>> +	 * This callback is optional.
>> +	 *
>> +	 * Returns:
>> +	 *
>> +	 * 0 on success or a negative error code on failure.
>> +	 */
>> +	int (*vmap_local)(struct dma_buf *dmabuf, struct dma_buf_map
>> *map);
>> +
>> +	/**
>> +	 * @vunmap_local:
>> +	 *
>> +	 * Removes a virtual mapping that was established by @vmap_local.
>> +	 *
>> +	 * This callback is optional.
>> +	 */
>> +	void (*vunmap_local)(struct dma_buf *dmabuf, struct dma_buf_map
>> *map);
>> };
>>
>> /**
>> @@ -501,4 +533,6 @@ int dma_buf_mmap(struct dma_buf *, struct
>> vm_area_struct *,
>> 		 unsigned long);
>> int dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map);
>> void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map
>> *map);
>> +int dma_buf_vmap_local(struct dma_buf *dmabuf, struct dma_buf_map
>> *map);
>> +void dma_buf_vunmap_local(struct dma_buf *dmabuf, struct
>> dma_buf_map *map);
>> #endif /* __DMA_BUF_H__ */
>> --
>> 2.29.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Michael J. Ruhl Jan. 14, 2021, 2:39 p.m. UTC | #3
>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>Thomas Zimmermann
>Sent: Tuesday, January 12, 2021 2:45 AM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>; sumit.semwal@linaro.org;
>christian.koenig@amd.com; airlied@redhat.com; daniel@ffwll.ch;
>maarten.lankhorst@linux.intel.com; mripard@kernel.org;
>kraxel@redhat.com; hdegoede@redhat.com; sean@poorly.run;
>eric@anholt.net; sam@ravnborg.org
>Cc: linaro-mm-sig@lists.linaro.org; Daniel Vetter <daniel.vetter@ffwll.ch>;
>virtualization@lists.linux-foundation.org; dri-devel@lists.freedesktop.org;
>linux-media@vger.kernel.org
>Subject: Re: [PATCH v4 01/13] dma-buf: Add vmap_local and vnumap_local
>operations
>
>Hi
>
>Am 08.01.21 um 17:12 schrieb Ruhl, Michael J:
>>> -----Original Message-----
>>> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>> Thomas Zimmermann
>>> Sent: Friday, January 8, 2021 4:43 AM
>>> To: sumit.semwal@linaro.org; christian.koenig@amd.com;
>>> airlied@redhat.com; daniel@ffwll.ch; maarten.lankhorst@linux.intel.com;
>>> mripard@kernel.org; kraxel@redhat.com; hdegoede@redhat.com;
>>> sean@poorly.run; eric@anholt.net; sam@ravnborg.org
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; dri-
>devel@lists.freedesktop.org;
>>> virtualization@lists.linux-foundation.org; linaro-mm-sig@lists.linaro.org;
>>> Thomas Zimmermann <tzimmermann@suse.de>; linux-
>>> media@vger.kernel.org
>>> Subject: [PATCH v4 01/13] dma-buf: Add vmap_local and vnumap_local
>>> operations
>>>
>>> The existing dma-buf calls dma_buf_vmap() and dma_buf_vunmap() are
>>> allowed to pin the buffer or acquire the buffer's reservation object
>>> lock.
>>>
>>> This is a problem for callers that only require a short-term mapping
>>> of the buffer without the pinning, or callers that have special locking
>>> requirements. These may suffer from unnecessary overhead or interfere
>>> with regular pin operations.
>>>
>>> The new interfaces dma_buf_vmap_local(), dma_buf_vunmapo_local(),
>and
>>> their rsp callbacks in struct dma_buf_ops provide an alternative without
>>> pinning or reservation locking. Callers are responsible for these
>>> operations.
>>>
>>> v4:
>>> 	* update documentation (Daniel)
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> ---
>>> drivers/dma-buf/dma-buf.c | 81
>>> +++++++++++++++++++++++++++++++++++++++
>>> include/linux/dma-buf.h   | 34 ++++++++++++++++
>>> 2 files changed, 115 insertions(+)
>>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index b8465243eca2..01f9c74d97fa 100644
>>> --- a/drivers/dma-buf/dma-buf.c
>>> +++ b/drivers/dma-buf/dma-buf.c
>>> @@ -1295,6 +1295,87 @@ void dma_buf_vunmap(struct dma_buf
>*dmabuf,
>>> struct dma_buf_map *map)
>>> }
>>> EXPORT_SYMBOL_GPL(dma_buf_vunmap);
>>>
>>> +/**
>>> + * dma_buf_vmap_local - Create virtual mapping for the buffer object
>into
>>> kernel
>>> + * address space.
>>> + * @dmabuf:	[in]	buffer to vmap
>>> + * @map:	[out]	returns the vmap pointer
>>> + *
>>> + * Unlike dma_buf_vmap() this is a short term mapping and will not pin
>>> + * the buffer. The struct dma_resv for the @dmabuf must be locked until
>>> + * dma_buf_vunmap_local() is called.
>>> + *
>>> + * Returns:
>>> + * 0 on success, or a negative errno code otherwise.
>>> + */
>>> +int dma_buf_vmap_local(struct dma_buf *dmabuf, struct dma_buf_map
>>> *map)
>>> +{
>>> +	struct dma_buf_map ptr;
>>> +	int ret = 0;
>>> +
>>> +	dma_buf_map_clear(map);
>>> +
>>> +	if (WARN_ON(!dmabuf))
>>> +		return -EINVAL;
>>> +
>>> +	dma_resv_assert_held(dmabuf->resv);
>>> +
>>> +	if (!dmabuf->ops->vmap_local)
>>> +		return -EINVAL;
>>
>> You are clearing the map, and then doing the above checks.
>>
>> Is it ok to change the map info and then exit on error?
>
>In vmap_local map argument returns the mapping's address. Callers are
>expected to check the return code. But I would expect a careless caller
>to not check, or check for map being cleared. Clearing it here first is
>the save thing to do.

Ahh, a pre-emptive attempt to make sure the caller doesn't cause more
problems on error. 
diff mbox series

Patch

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index b8465243eca2..01f9c74d97fa 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1295,6 +1295,87 @@  void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map)
 }
 EXPORT_SYMBOL_GPL(dma_buf_vunmap);
 
+/**
+ * dma_buf_vmap_local - Create virtual mapping for the buffer object into kernel
+ * address space.
+ * @dmabuf:	[in]	buffer to vmap
+ * @map:	[out]	returns the vmap pointer
+ *
+ * Unlike dma_buf_vmap() this is a short term mapping and will not pin
+ * the buffer. The struct dma_resv for the @dmabuf must be locked until
+ * dma_buf_vunmap_local() is called.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int dma_buf_vmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map)
+{
+	struct dma_buf_map ptr;
+	int ret = 0;
+
+	dma_buf_map_clear(map);
+
+	if (WARN_ON(!dmabuf))
+		return -EINVAL;
+
+	dma_resv_assert_held(dmabuf->resv);
+
+	if (!dmabuf->ops->vmap_local)
+		return -EINVAL;
+
+	mutex_lock(&dmabuf->lock);
+	if (dmabuf->vmapping_counter) {
+		dmabuf->vmapping_counter++;
+		BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr));
+		*map = dmabuf->vmap_ptr;
+		goto out_unlock;
+	}
+
+	BUG_ON(dma_buf_map_is_set(&dmabuf->vmap_ptr));
+
+	ret = dmabuf->ops->vmap_local(dmabuf, &ptr);
+	if (WARN_ON_ONCE(ret))
+		goto out_unlock;
+
+	dmabuf->vmap_ptr = ptr;
+	dmabuf->vmapping_counter = 1;
+
+	*map = dmabuf->vmap_ptr;
+
+out_unlock:
+	mutex_unlock(&dmabuf->lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dma_buf_vmap_local);
+
+/**
+ * dma_buf_vunmap_local - Unmap a vmap obtained by dma_buf_vmap_local.
+ * @dmabuf:	[in]	buffer to vunmap
+ * @map:	[in]	vmap pointer to vunmap
+ *
+ * Release a mapping established with dma_buf_vmap_local().
+ */
+void dma_buf_vunmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map)
+{
+	if (WARN_ON(!dmabuf))
+		return;
+
+	dma_resv_assert_held(dmabuf->resv);
+
+	BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr));
+	BUG_ON(dmabuf->vmapping_counter == 0);
+	BUG_ON(!dma_buf_map_is_equal(&dmabuf->vmap_ptr, map));
+
+	mutex_lock(&dmabuf->lock);
+	if (--dmabuf->vmapping_counter == 0) {
+		if (dmabuf->ops->vunmap_local)
+			dmabuf->ops->vunmap_local(dmabuf, map);
+		dma_buf_map_clear(&dmabuf->vmap_ptr);
+	}
+	mutex_unlock(&dmabuf->lock);
+}
+EXPORT_SYMBOL_GPL(dma_buf_vunmap_local);
+
 #ifdef CONFIG_DEBUG_FS
 static int dma_buf_debug_show(struct seq_file *s, void *unused)
 {
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 628681bf6c99..aeed754b5467 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -264,6 +264,38 @@  struct dma_buf_ops {
 
 	int (*vmap)(struct dma_buf *dmabuf, struct dma_buf_map *map);
 	void (*vunmap)(struct dma_buf *dmabuf, struct dma_buf_map *map);
+
+	/**
+	 * @vmap_local:
+	 *
+	 * Creates a virtual mapping for the buffer into kernel address space.
+	 *
+	 * This callback establishes short-term mappings for situations where
+	 * callers only use the buffer for a bounded amount of time; such as
+	 * updates to the framebuffer or reading back contained information.
+	 * In contrast to the regular @vmap callback, vmap_local does never pin
+	 * the buffer to a specific domain or acquire the buffer's reservation
+	 * lock.
+	 *
+	 * This is called with the &dma_buf.resv object locked. Callers must hold
+	 * the lock until after removing the mapping with @vunmap_local.
+	 *
+	 * This callback is optional.
+	 *
+	 * Returns:
+	 *
+	 * 0 on success or a negative error code on failure.
+	 */
+	int (*vmap_local)(struct dma_buf *dmabuf, struct dma_buf_map *map);
+
+	/**
+	 * @vunmap_local:
+	 *
+	 * Removes a virtual mapping that was established by @vmap_local.
+	 *
+	 * This callback is optional.
+	 */
+	void (*vunmap_local)(struct dma_buf *dmabuf, struct dma_buf_map *map);
 };
 
 /**
@@ -501,4 +533,6 @@  int dma_buf_mmap(struct dma_buf *, struct vm_area_struct *,
 		 unsigned long);
 int dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map);
 void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map);
+int dma_buf_vmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map);
+void dma_buf_vunmap_local(struct dma_buf *dmabuf, struct dma_buf_map *map);
 #endif /* __DMA_BUF_H__ */