From: Rob Clark <robdclark(a)chromium.org>
This is a re-post of the remaining patches from:
https://patchwork.freedesktop.org/series/114490/
Part of the hold-up of the remaining uabi patches was compositor
support, but now an MR for kwin exists:
https://invent.kde.org/plasma/kwin/-/merge_requests/4358
The syncobj userspace is:
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21973
v1: https://patchwork.freedesktop.org/series/93035/
v2: Move filtering out of later deadlines to fence implementation
to avoid increasing the size of dma_fence
v3: Add support in fence-array and fence-chain; Add some uabi to
support igt tests and userspace compositors.
v4: Rebase, address various comments, and add syncobj deadline
support, and sync_file EPOLLPRI based on experience with perf/
freq issues with clvk compute workloads on i915 (anv)
v5: Clarify that this is a hint as opposed to a more hard deadline
guarantee, switch to using u64 ns values in UABI (still absolute
CLOCK_MONOTONIC values), drop syncobj related cap and driver
feature flag in favor of allowing count_handles==0 for probing
kernel support.
v6: Re-work vblank helper to calculate time of _start_ of vblank,
and work correctly if the last vblank event was more than a
frame ago. Add (mostly unrelated) drm/msm patch which also
uses the vblank helper. Use dma_fence_chain_contained(). More
verbose syncobj UABI comments. Drop DMA_FENCE_FLAG_HAS_DEADLINE_BIT.
v7: Fix kbuild complaints about vblank helper. Add more docs.
v8: Add patch to surface sync_file UAPI, and more docs updates.
v9: Repost the remaining patches that expose new uabi to userspace.
Rob Clark (3):
drm/syncobj: Add deadline support for syncobj waits
dma-buf/sync_file: Add SET_DEADLINE ioctl
dma-buf/sw_sync: Add fence deadline support
drivers/dma-buf/dma-fence.c | 3 +-
drivers/dma-buf/sw_sync.c | 82 ++++++++++++++++++++++++++++++++++
drivers/dma-buf/sync_debug.h | 2 +
drivers/dma-buf/sync_file.c | 19 ++++++++
drivers/gpu/drm/drm_syncobj.c | 64 ++++++++++++++++++++------
include/uapi/drm/drm.h | 17 +++++++
include/uapi/linux/sync_file.h | 22 +++++++++
7 files changed, 195 insertions(+), 14 deletions(-)
--
2.41.0
On Thu, Nov 30, 2023 at 06:14:16PM +0100, Marco Pagani wrote:
> This patch introduces an initial KUnit test suite for GEM objects
> backed by shmem buffers.
>
> Suggested-by: Javier Martinez Canillas <javierm(a)redhat.com>
> Signed-off-by: Marco Pagani <marpagan(a)redhat.com>
>
> v5:
> - using __drm_kunit_helper_alloc_drm_device() to avoid local struct
> v4:
> - Add missing MMU dependency for DRM_GEM_SHMEM_HELPER (kernel test robot)
> v3:
> - Explicitly cast pointers in the helpers
> - Removed unused pointer to parent dev in struct fake_dev
> - Test entries reordering in Kconfig and Makefile sent as a separate patch
> v2:
> - Improved description of test cases
> - Cleaner error handling using KUnit actions
> - Alphabetical order in Kconfig and Makefile
Applied to drm-misc-next, thanks!
Maxime
Am 22.11.23 um 17:05 schrieb Ramesh Errabolu:
> Fix the documentation of struct dma_buf members name and exp_name
> as to how these members are to be used and accessed.
>
> Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu(a)amd.com>
Reviewed-by: Christian König <christian.koenig(a)amd.com>
> ---
> include/linux/dma-buf.h | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 3f31baa3293f..8ff4add71f88 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -343,16 +343,19 @@ struct dma_buf {
> /**
> * @exp_name:
> *
> - * Name of the exporter; useful for debugging. See the
> - * DMA_BUF_SET_NAME IOCTL.
> + * Name of the exporter; useful for debugging. Must not be NULL
> */
> const char *exp_name;
>
> /**
> * @name:
> *
> - * Userspace-provided name; useful for accounting and debugging,
> - * protected by dma_resv_lock() on @resv and @name_lock for read access.
> + * Userspace-provided name. Default value is NULL. If not NULL,
> + * length cannot be longer than DMA_BUF_NAME_LEN, including NIL
> + * char. Useful for accounting and debugging. Read/Write accesses
> + * are protected by @name_lock
> + *
> + * See the IOCTLs DMA_BUF_SET_NAME or DMA_BUF_SET_NAME_A/B
> */
> const char *name;
>
On Fri, Nov 24, 2023 at 11:15:12AM +0100, Marco Pagani wrote:
>
>
> On 2023-11-24 09:49, Maxime Ripard wrote:
> > Hi,
> >
> > On Thu, Nov 23, 2023 at 11:01:46AM +0100, Marco Pagani wrote:
> >> +static int drm_gem_shmem_test_init(struct kunit *test)
> >> +{
> >> + struct device *dev;
> >> + struct fake_dev {
> >> + struct drm_device drm_dev;
> >> + } *fdev;
> >> +
> >
> > [...]
> >
> >> +
> >> + /*
> >> + * The DRM core will automatically initialize the GEM core and create
> >> + * a DRM Memory Manager object which provides an address space pool
> >> + * for GEM objects allocation.
> >> + */
> >> + fdev = drm_kunit_helper_alloc_drm_device(test, dev, struct fake_dev,
> >> + drm_dev, DRIVER_GEM);
> >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fdev);
> >
> > Sorry I missed it earlier, but you don't need the intermediate structure
> > if you use
> >
> > struct drm_device *drm;
> >
> > drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0, DRIVER_GEM);
> > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
> >
>
> I prefer to use drm_kunit_helper_alloc_drm_device() with the intermediate
> structure. It makes the code clearer, in my opinion. Initially, when
> developing the suite, I was using __drm_kunit_helper_alloc_drm_device()
> as most test suites do, but I feel the list of arguments including
> "sizeof(*drm), 0," is less straightforward to understand.
Then we can create an init helper, and you can allocate the drm_device
through drmm_kzalloc, but I'd like tests to use consistent constructs.
This can of course be made as a later patch: you use the same construct
the other tests do here, and later we work on the init function and
convert all tests to use it.
Maxime
It's valid to add the same fence multiple times to a dma-resv object and
we shouldn't need one extra slot for each.
Signed-off-by: Christian König <christian.koenig(a)amd.com>
Fixes: a3f7c10a269d5 ("dma-buf/dma-resv: check if the new fence is really later")
Cc: stable(a)vger.kernel.org # v5.19+
---
drivers/dma-buf/dma-resv.c | 2 +-
include/linux/dma-fence.h | 15 +++++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index 38b4110378de..eb8b733065b2 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -301,7 +301,7 @@ void dma_resv_add_fence(struct dma_resv *obj, struct dma_fence *fence,
dma_resv_list_entry(fobj, i, obj, &old, &old_usage);
if ((old->context == fence->context && old_usage >= usage &&
- dma_fence_is_later(fence, old)) ||
+ dma_fence_is_later_or_same(fence, old)) ||
dma_fence_is_signaled(old)) {
dma_resv_list_set(fobj, i, fence, usage);
dma_fence_put(old);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index ebe78bd3d121..b3772edca2e6 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -498,6 +498,21 @@ static inline bool dma_fence_is_later(struct dma_fence *f1,
return __dma_fence_is_later(f1->seqno, f2->seqno, f1->ops);
}
+/**
+ * dma_fence_is_later_or_same - return true if f1 is later or same as f2
+ * @f1: the first fence from the same context
+ * @f2: the second fence from the same context
+ *
+ * Returns true if f1 is chronologically later than f2 or the same fence. Both
+ * fences must be from the same context, since a seqno is not re-used across
+ * contexts.
+ */
+static inline bool dma_fence_is_later_or_same(struct dma_fence *f1,
+ struct dma_fence *f2)
+{
+ return f1 == f2 || dma_fence_is_later(f1, f2);
+}
+
/**
* dma_fence_later - return the chronologically later fence
* @f1: the first fence from the same context
--
2.34.1
strlcpy() reads the entire source buffer first. This read may exceed
the destination size limit. This is both inefficient and can lead
to linear read overflows if a source string is not NUL-terminated[1].
Additionally, it returns the size of the source string, not the
resulting size of the destination string. In an effort to remove strlcpy()
completely[2], replace strlcpy() here with strscpy().
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [1]
Link: https://github.com/KSPP/linux/issues/89 [2]
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: "Christian König" <christian.koenig(a)amd.com>
Cc: Azeem Shaikh <azeemshaikh38(a)gmail.com>
Cc: linux-media(a)vger.kernel.org
Cc: dri-devel(a)lists.freedesktop.org
Cc: linaro-mm-sig(a)lists.linaro.org
Signed-off-by: Kees Cook <keescook(a)chromium.org>
---
drivers/dma-buf/dma-buf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 21916bba77d5..8fe5aa67b167 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -46,12 +46,12 @@ static char *dmabuffs_dname(struct dentry *dentry, char *buffer, int buflen)
{
struct dma_buf *dmabuf;
char name[DMA_BUF_NAME_LEN];
- size_t ret = 0;
+ ssize_t ret = 0;
dmabuf = dentry->d_fsdata;
spin_lock(&dmabuf->name_lock);
if (dmabuf->name)
- ret = strlcpy(name, dmabuf->name, DMA_BUF_NAME_LEN);
+ ret = strscpy(name, dmabuf->name, sizeof(name));
spin_unlock(&dmabuf->name_lock);
return dynamic_dname(buffer, buflen, "/%s:%s",
--
2.34.1