[PATCH 1/2] libcamera: v4l2_device: Retry ::ioctl on EINTR

[Kieran Bingham]

Hi Umang,

Quoting Umang Jain (2024-08-02 08:24:15)
> V4L userspace API documentation clearly mentions the handling of
> EINTR on ioctl(). Align with the xioctl() reference provided in [1].
> 
> Retry the ::ioctl() if failed with errno == EINTR. Use a do..while{}
> to check the return value of ::ioctl() and errno value. If the return
> value is found to be -1, and errno is set with EINTR, the ioctl() call
> should be retried.
> 
> [1]: https://docs.kernel.org/userspace-api/media/v4l/capture.c.html
> 

I haven't checked where, but I presume any of the operations that can be
long running or blocking in the kernel such as DQ_BUF would have
interruptible locks, which would make this important to have.

Because we use poll/select to only call those when we know there will be
something available, I expect that the chance of hitting this is
incredibly low, - but I do believe it's more correct to ensure we handle
this.

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/libcamera/v4l2_device.cpp | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 4a2048cf..2e92db0f 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -454,11 +454,17 @@ int V4L2Device::setFrameStartEnabled(bool enable)
>   */
>  int V4L2Device::ioctl(unsigned long request, void *argp)
>  {
> +       int ret;
> +
> +       do {
> +               ret = ::ioctl(fd_.get(), request, argp);
> +       } while (ret == -1 && errno == EINTR);
> +
>         /*
>          * Printing out an error message is usually better performed
>          * in the caller, which can provide more context.
>          */
> -       if (::ioctl(fd_.get(), request, argp) < 0)
> +       if (ret < 0)
>                 return -errno;
>  
>         return 0;
> -- 
> 2.45.2
>