On 26/06/2020 08:02, Hans Verkuil wrote: > Hi Guillaume, > > You need to update v4l-utils to the latest version from our git master branch. > > The reserved field in reqbufs is now in use as a flags field, so it is no longer > zero. The compliance test has been updated accordingly in the v4l-utils git repo. I see, thanks. It's getting updated today. Guillaume > On 26/06/2020 08:56, Guillaume Tucker wrote: >> Please see the bisection report below about a regression in >> v4l2-compliance with uvcvideo: >> >> [ 25.495039] uvcvideo: Failed to query (SET_CUR) UVC control 10 on unit 2: -32 (exp. 2). >> fail: v4l2-test-buffers.cpp(680): check_0(reqbufs.reserved, sizeof(reqbufs.reserved)) >> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL >> >> >> as seen in the full job log: >> >> https://storage.kernelci.org/media/master/v5.8-rc1-64-ge30cc79cc80f/arm64/defconfig/gcc-8/lab-collabora/v4l2-compliance-uvc-rk3399-gru-kevin.html#L1713 >> >> with a few more details about the regression here: >> >> https://kernelci.org/test/case/id/5ef23169140826f73d97bf51/ >> >> and the same test case failure also seen with vivid: >> >> https://kernelci.org/test/case/id/5ef23699f641f7b3e597bf3f/ >> >> >> The bisection actually ran a couple of days ago but there was an >> email error when sending the report, so I'm sending it by hand >> now. I hope the issue hasn't spread too widely already, I know >> it's also affecting linux-next. >> >> Guillaume >> >> >> On 25/06/2020 23:19, kernelci.org bot wrote: >>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * >>> * This automated bisection report was sent to you on the basis * >>> * that you may be involved with the breaking commit it has * >>> * found. No manual investigation has been done to verify it, * >>> * and the root cause of the problem may be somewhere else. * >>> * * >>> * If you do send a fix, please include this trailer: * >>> * Reported-by: "kernelci.org bot" <bot@xxxxxxxxxxxx> * >>> * * >>> * Hope this helps! * >>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * >>> >>> media/master bisection: v4l2-compliance-uvc.Buffer-ioctls-Input-0.VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF on rk3399-gru-kevin >>> >>> Summary: >>> Start: e30cc79cc80f media: media-request: Fix crash if memory allocation fails >>> Plain log: https://storage.kernelci.org/media/master/v5.8-rc1-64-ge30cc79cc80f/arm64/defconfig/gcc-8/lab-collabora/v4l2-compliance-uvc-rk3399-gru-kevin.txt >>> HTML log: https://storage.kernelci.org/media/master/v5.8-rc1-64-ge30cc79cc80f/arm64/defconfig/gcc-8/lab-collabora/v4l2-compliance-uvc-rk3399-gru-kevin.html >>> Result: 1e0b2318fa75 media: videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag >>> >>> Checks: >>> revert: PASS >>> verify: PASS >>> >>> Parameters: >>> Tree: media >>> URL: https://git.linuxtv.org/media_tree.git >>> Branch: master >>> Target: rk3399-gru-kevin >>> CPU arch: arm64 >>> Lab: lab-collabora >>> Compiler: gcc-8 >>> Config: defconfig >>> Test case: v4l2-compliance-uvc.Buffer-ioctls-Input-0.VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF >>> >>> Breaking commit found: >>> >>> ------------------------------------------------------------------------------- >>> commit 1e0b2318fa75d186ee0d2be31843ce867385fcc4 >>> Author: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx> >>> Date: Thu May 14 18:01:45 2020 +0200 >>> >>> media: videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag >>> >>> This patch lets user-space to request a non-consistent memory >>> allocation during CREATE_BUFS and REQBUFS ioctl calls. >>> >>> = CREATE_BUFS >>> >>> struct v4l2_create_buffers has seven 4-byte reserved areas, >>> so reserved[0] is renamed to ->flags. The struct, thus, now >>> has six reserved 4-byte regions. >>> >>> = CREATE_BUFS32 >>> >>> struct v4l2_create_buffers32 has seven 4-byte reserved areas, >>> so reserved[0] is renamed to ->flags. The struct, thus, now >>> has six reserved 4-byte regions. >>> >>> = REQBUFS >>> >>> We use one bit of a ->reserved[1] member of struct v4l2_requestbuffers, >>> which is now renamed to ->flags. Unlike v4l2_create_buffers, struct >>> v4l2_requestbuffers does not have enough reserved room. Therefore for >>> backward compatibility ->reserved and ->flags were put into anonymous >>> union. >>> >>> Signed-off-by: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx> >>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> >>> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> >>> >>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst >>> index e1afc5b504c2..f2a702870fad 100644 >>> --- a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst >>> +++ b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst >>> @@ -121,7 +121,12 @@ than the number requested. >>> other changes, then set ``count`` to 0, ``memory`` to >>> ``V4L2_MEMORY_MMAP`` and ``format.type`` to the buffer type. >>> * - __u32 >>> - - ``reserved``\ [7] >>> + - ``flags`` >>> + - Specifies additional buffer management attributes. >>> + See :ref:`memory-flags`. >>> + >>> + * - __u32 >>> + - ``reserved``\ [6] >>> - A place holder for future extensions. Drivers and applications >>> must set the array to zero. >>> >>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst >>> index 96a59793d857..75d894d9c36c 100644 >>> --- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst >>> +++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst >>> @@ -112,10 +112,17 @@ aborting or finishing any DMA in progress, an implicit >>> ``V4L2_MEMORY_MMAP`` and ``type`` set to the buffer type. This will >>> free any previously allocated buffers, so this is typically something >>> that will be done at the start of the application. >>> + * - union { >>> + - (anonymous) >>> + * - __u32 >>> + - ``flags`` >>> + - Specifies additional buffer management attributes. >>> + See :ref:`memory-flags`. >>> * - __u32 >>> - ``reserved``\ [1] >>> - - A place holder for future extensions. Drivers and applications >>> - must set the array to zero. >>> + - Kept for backwards compatibility. Use ``flags`` instead. >>> + * - } >>> + - >>> >>> .. tabularcolumns:: |p{6.1cm}|p{2.2cm}|p{8.7cm}| >>> >>> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c >>> index 0fdcf90330df..626c4db5134c 100644 >>> --- a/drivers/media/common/videobuf2/videobuf2-core.c >>> +++ b/drivers/media/common/videobuf2/videobuf2-core.c >>> @@ -694,6 +694,9 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, >>> unsigned int i; >>> int ret; >>> >>> + if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) >>> + consistent_mem = false; >>> + >>> if (q->streaming) { >>> dprintk(1, "streaming active\n"); >>> return -EBUSY; >>> @@ -837,6 +840,9 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, >>> bool consistent_mem = true; >>> int ret; >>> >>> + if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) >>> + consistent_mem = false; >>> + >>> if (q->num_buffers == VB2_MAX_FRAME) { >>> dprintk(1, "maximum number of buffers already allocated\n"); >>> return -ENOBUFS; >>> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c >>> index 26a3ec333bb7..559a229cac41 100644 >>> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c >>> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c >>> @@ -718,12 +718,22 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps) >>> #endif >>> } >>> >>> +static void clear_consistency_attr(struct vb2_queue *q, >>> + int memory, >>> + unsigned int *flags) >>> +{ >>> + if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP) >>> + *flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT; >>> +} >>> + >>> int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) >>> { >>> int ret = vb2_verify_memory_type(q, req->memory, req->type); >>> >>> fill_buf_caps(q, &req->capabilities); >>> - return ret ? ret : vb2_core_reqbufs(q, req->memory, 0, &req->count); >>> + clear_consistency_attr(q, req->memory, &req->flags); >>> + return ret ? ret : vb2_core_reqbufs(q, req->memory, >>> + req->flags, &req->count); >>> } >>> EXPORT_SYMBOL_GPL(vb2_reqbufs); >>> >>> @@ -755,6 +765,7 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) >>> unsigned i; >>> >>> fill_buf_caps(q, &create->capabilities); >>> + clear_consistency_attr(q, create->memory, &create->flags); >>> create->index = q->num_buffers; >>> if (create->count == 0) >>> return ret != -EBUSY ? ret : 0; >>> @@ -797,8 +808,11 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) >>> for (i = 0; i < requested_planes; i++) >>> if (requested_sizes[i] == 0) >>> return -EINVAL; >>> - return ret ? ret : vb2_core_create_bufs(q, create->memory, 0, >>> - &create->count, requested_planes, requested_sizes); >>> + return ret ? ret : vb2_core_create_bufs(q, create->memory, >>> + create->flags, >>> + &create->count, >>> + requested_planes, >>> + requested_sizes); >>> } >>> EXPORT_SYMBOL_GPL(vb2_create_bufs); >>> >>> @@ -969,11 +983,12 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv, >>> int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type); >>> >>> fill_buf_caps(vdev->queue, &p->capabilities); >>> + clear_consistency_attr(vdev->queue, p->memory, &p->flags); >>> if (res) >>> return res; >>> if (vb2_queue_is_busy(vdev, file)) >>> return -EBUSY; >>> - res = vb2_core_reqbufs(vdev->queue, p->memory, 0, &p->count); >>> + res = vb2_core_reqbufs(vdev->queue, p->memory, p->flags, &p->count); >>> /* If count == 0, then the owner has released all buffers and he >>> is no longer owner of the queue. Otherwise we have a new owner. */ >>> if (res == 0) >>> @@ -991,6 +1006,7 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv, >>> >>> p->index = vdev->queue->num_buffers; >>> fill_buf_caps(vdev->queue, &p->capabilities); >>> + clear_consistency_attr(vdev->queue, p->memory, &p->flags); >>> /* >>> * If count == 0, then just check if memory and type are valid. >>> * Any -EBUSY result from vb2_verify_memory_type can be mapped to 0. >>> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c >>> index a99e82ec9ab6..593bcf6c3735 100644 >>> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c >>> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c >>> @@ -246,6 +246,9 @@ struct v4l2_format32 { >>> * @memory: buffer memory type >>> * @format: frame format, for which buffers are requested >>> * @capabilities: capabilities of this buffer type. >>> + * @flags: additional buffer management attributes (ignored unless the >>> + * queue has V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS capability and >>> + * configured for MMAP streaming I/O). >>> * @reserved: future extensions >>> */ >>> struct v4l2_create_buffers32 { >>> @@ -254,7 +257,8 @@ struct v4l2_create_buffers32 { >>> __u32 memory; /* enum v4l2_memory */ >>> struct v4l2_format32 format; >>> __u32 capabilities; >>> - __u32 reserved[7]; >>> + __u32 flags; >>> + __u32 reserved[6]; >>> }; >>> >>> static int __bufsize_v4l2_format(struct v4l2_format32 __user *p32, u32 *size) >>> @@ -355,7 +359,8 @@ static int get_v4l2_create32(struct v4l2_create_buffers __user *p64, >>> { >>> if (!access_ok(p32, sizeof(*p32)) || >>> copy_in_user(p64, p32, >>> - offsetof(struct v4l2_create_buffers32, format))) >>> + offsetof(struct v4l2_create_buffers32, format)) || >>> + assign_in_user(&p64->flags, &p32->flags)) >>> return -EFAULT; >>> return __get_v4l2_format32(&p64->format, &p32->format, >>> aux_buf, aux_space); >>> @@ -417,6 +422,7 @@ static int put_v4l2_create32(struct v4l2_create_buffers __user *p64, >>> copy_in_user(p32, p64, >>> offsetof(struct v4l2_create_buffers32, format)) || >>> assign_in_user(&p32->capabilities, &p64->capabilities) || >>> + assign_in_user(&p32->flags, &p64->flags) || >>> copy_in_user(p32->reserved, p64->reserved, sizeof(p64->reserved))) >>> return -EFAULT; >>> return __put_v4l2_format32(&p64->format, &p32->format); >>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c >>> index 2322f08a98be..02bfef0da76d 100644 >>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c >>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c >>> @@ -2038,9 +2038,6 @@ static int v4l_reqbufs(const struct v4l2_ioctl_ops *ops, >>> >>> if (ret) >>> return ret; >>> - >>> - CLEAR_AFTER_FIELD(p, capabilities); >>> - >>> return ops->vidioc_reqbufs(file, fh, p); >>> } >>> >>> @@ -2080,7 +2077,7 @@ static int v4l_create_bufs(const struct v4l2_ioctl_ops *ops, >>> if (ret) >>> return ret; >>> >>> - CLEAR_AFTER_FIELD(create, capabilities); >>> + CLEAR_AFTER_FIELD(create, flags); >>> >>> v4l_sanitize_format(&create->format); >>> >>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h >>> index 34ba1017b89b..fec2607a07e3 100644 >>> --- a/include/uapi/linux/videodev2.h >>> +++ b/include/uapi/linux/videodev2.h >>> @@ -946,7 +946,10 @@ struct v4l2_requestbuffers { >>> __u32 type; /* enum v4l2_buf_type */ >>> __u32 memory; /* enum v4l2_memory */ >>> __u32 capabilities; >>> - __u32 reserved[1]; >>> + union { >>> + __u32 flags; >>> + __u32 reserved[1]; >>> + }; >>> }; >>> >>> /* capabilities for struct v4l2_requestbuffers and v4l2_create_buffers */ >>> @@ -2450,6 +2453,9 @@ struct v4l2_dbg_chip_info { >>> * @memory: enum v4l2_memory; buffer memory type >>> * @format: frame format, for which buffers are requested >>> * @capabilities: capabilities of this buffer type. >>> + * @flags: additional buffer management attributes (ignored unless the >>> + * queue has V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS capability >>> + * and configured for MMAP streaming I/O). >>> * @reserved: future extensions >>> */ >>> struct v4l2_create_buffers { >>> @@ -2458,7 +2464,8 @@ struct v4l2_create_buffers { >>> __u32 memory; >>> struct v4l2_format format; >>> __u32 capabilities; >>> - __u32 reserved[7]; >>> + __u32 flags; >>> + __u32 reserved[6]; >>> }; >>> >>> /* >>> ------------------------------------------------------------------------------- >>> >>> >>> Git bisection log: >>> >>> ------------------------------------------------------------------------------- >>> git bisect start >>> # good: [2630e1bb0948c3134c6f22ad275ae27cc6023532] media: rkvdec: Fix H264 scaling list order >>> git bisect good 2630e1bb0948c3134c6f22ad275ae27cc6023532 >>> # bad: [e30cc79cc80fd919b697a15c5000d9f57487de8e] media: media-request: Fix crash if memory allocation fails >>> git bisect bad e30cc79cc80fd919b697a15c5000d9f57487de8e >>> # good: [cb8e59cc87201af93dfbb6c3dccc8fcad72a09c2] Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next >>> git bisect good cb8e59cc87201af93dfbb6c3dccc8fcad72a09c2 >>> # good: [3b69e8b4571125bec1f77f886174fe6cab6b9d75] Merge tag 'sh-for-5.8' of git://git.libc.org/linux-sh >>> git bisect good 3b69e8b4571125bec1f77f886174fe6cab6b9d75 >>> # good: [3ed740841bf94a8028ec44164d84f9af9bd552fd] maccess: remove duplicate kerneldoc comments >>> git bisect good 3ed740841bf94a8028ec44164d84f9af9bd552fd >>> # good: [2ab70319bc1f79228da4dce7b9d604740c9beeef] nmi, tracing: Make hardware latency tracing noinstr safe >>> git bisect good 2ab70319bc1f79228da4dce7b9d604740c9beeef >>> # good: [df2fbf5bfa0e7fff8b4784507e4d68f200454318] Merge tag 'thermal-v5.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux >>> git bisect good df2fbf5bfa0e7fff8b4784507e4d68f200454318 >>> # good: [91fa58840ae22cbf6d7c505ce6564c4c48f29af3] Merge branch 'i2c/for-5.8' of git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux >>> git bisect good 91fa58840ae22cbf6d7c505ce6564c4c48f29af3 >>> # good: [bc139119a1708ae3db1ebb379630f286e28d06e8] net: ethernet: ti: ale: fix allmulti for nu type ale >>> git bisect good bc139119a1708ae3db1ebb379630f286e28d06e8 >>> # good: [157282a5c6273976b31cf4ba4b1c46663f6519d1] media: s5k6a3: Fix runtime PM imbalance on error >>> git bisect good 157282a5c6273976b31cf4ba4b1c46663f6519d1 >>> # bad: [286cf7d3a99e1ca8c1d8e674b9a98f2dbe8520dc] media: videodev2.h: add V4L2_FMT_FLAG_ENC_CAP_FRAME_INTERVAL flag >>> git bisect bad 286cf7d3a99e1ca8c1d8e674b9a98f2dbe8520dc >>> # good: [ac53503ee38a1ffbc47c7cca6cbfc48ba9c65c5e] media: videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag >>> git bisect good ac53503ee38a1ffbc47c7cca6cbfc48ba9c65c5e >>> # bad: [d4db5eb57cab049d378fbfb7ee842857009a8679] media: videobuf2: add begin/end cpu_access callbacks to dma-sg >>> git bisect bad d4db5eb57cab049d378fbfb7ee842857009a8679 >>> # bad: [38a417e88aad1e5ad5ff2647d4cd26040b567fc3] media: videobuf2: factor out planes prepare/finish functions >>> git bisect bad 38a417e88aad1e5ad5ff2647d4cd26040b567fc3 >>> # bad: [1e0b2318fa75d186ee0d2be31843ce867385fcc4] media: videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag >>> git bisect bad 1e0b2318fa75d186ee0d2be31843ce867385fcc4 >>> # good: [7b4b45555c79db03dad8192e6ef85cb30236827b] media: videobuf2: add queue memory consistency parameter >>> git bisect good 7b4b45555c79db03dad8192e6ef85cb30236827b >>> # first bad commit: [1e0b2318fa75d186ee0d2be31843ce867385fcc4] media: videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag >>> ------------------------------------------------------------------------------- >>> >> >
