lkml.org 
[lkml]   [2012]   [Dec]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[Regression w/ patch] Media commit causes user space to misbahave (was: Re: Linux 3.8-rc1)
    Date
    Hi Laurent,

    Commit f0ed2ce ([media] uvcvideo: Set error_idx properly for extended
    controls API failures) of yours unfortunately causes user space to misbehave
    on one of my test systems. Namely, there's no sound under KDE 4.9.4
    (Tumbleweed version using pulseaudio) and there's a knotify4 process occupying
    one of the CPU cores entirely 100% of the time.

    I used the patch below to fix the problem for me.

    Thanks,
    Rafael


    ---
    From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
    Subject: [media]: Revert part of commit f0ed2ce to fix user space breakage

    Commit f0ed2ce ([media] uvcvideo: Set error_idx properly for extended
    controls API failures) causes user space to behave incorrectly on one
    of my test machines (there is no sound under KDE 4.9.4 using
    pulseaudio and there is a knotify4 process occupying one of the CPU
    cores 100% of the time). Reverting that commit entirely fixes the
    problem for me.

    However, commit f0ed2ce appears to do more than it follows from its
    changelog, because the changelog only says about the changes related
    to ctrls->error_idx, while the commit additionally changes error
    codes returned by various functions in uvc_ctrl.c and uvc_v4l2.c. It
    turns out that the changes of the returned error codes confuse the
    user spce, so it is sufficient to revert the part of commit f0ed2ce
    not mentioned in its changelog to fix the problem.

    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
    ---
    drivers/media/usb/uvc/uvc_ctrl.c | 19 ++++++++-----------
    drivers/media/usb/uvc/uvc_v4l2.c | 12 +++++-------
    2 files changed, 13 insertions(+), 18 deletions(-)

    Index: linux/drivers/media/usb/uvc/uvc_ctrl.c
    ===================================================================
    --- linux.orig/drivers/media/usb/uvc/uvc_ctrl.c
    +++ linux/drivers/media/usb/uvc/uvc_ctrl.c
    @@ -1061,7 +1061,7 @@ int uvc_query_v4l2_ctrl(struct uvc_video

    ctrl = uvc_find_control(chain, v4l2_ctrl->id, &mapping);
    if (ctrl == NULL) {
    - ret = -ENOENT;
    + ret = -EINVAL;
    goto done;
    }

    @@ -1099,13 +1099,12 @@ int uvc_query_v4l2_menu(struct uvc_video
    return -ERESTARTSYS;

    ctrl = uvc_find_control(chain, query_menu->id, &mapping);
    - if (ctrl == NULL) {
    - ret = -ENOENT;
    + if (ctrl == NULL || mapping->v4l2_type != V4L2_CTRL_TYPE_MENU) {
    + ret = -EINVAL;
    goto done;
    }

    - if (mapping->v4l2_type != V4L2_CTRL_TYPE_MENU ||
    - query_menu->index >= mapping->menu_count) {
    + if (query_menu->index >= mapping->menu_count) {
    ret = -EINVAL;
    goto done;
    }
    @@ -1264,7 +1263,7 @@ static int uvc_ctrl_add_event(struct v4l

    ctrl = uvc_find_control(handle->chain, sev->id, &mapping);
    if (ctrl == NULL) {
    - ret = -ENOENT;
    + ret = -EINVAL;
    goto done;
    }

    @@ -1415,7 +1414,7 @@ int uvc_ctrl_get(struct uvc_video_chain

    ctrl = uvc_find_control(chain, xctrl->id, &mapping);
    if (ctrl == NULL)
    - return -ENOENT;
    + return -EINVAL;

    return __uvc_ctrl_get(chain, ctrl, mapping, &xctrl->value);
    }
    @@ -1432,10 +1431,8 @@ int uvc_ctrl_set(struct uvc_video_chain
    int ret;

    ctrl = uvc_find_control(chain, xctrl->id, &mapping);
    - if (ctrl == NULL)
    - return -ENOENT;
    - if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
    - return -EACCES;
    + if (ctrl == NULL || (ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR) == 0)
    + return -EINVAL;

    /* Clamp out of range values. */
    switch (mapping->v4l2_type) {
    Index: linux/drivers/media/usb/uvc/uvc_v4l2.c
    ===================================================================
    --- linux.orig/drivers/media/usb/uvc/uvc_v4l2.c
    +++ linux/drivers/media/usb/uvc/uvc_v4l2.c
    @@ -607,10 +607,8 @@ static long uvc_v4l2_do_ioctl(struct fil

    ret = uvc_ctrl_get(chain, &xctrl);
    uvc_ctrl_rollback(handle);
    - if (ret < 0)
    - return ret == -ENOENT ? -EINVAL : ret;
    -
    - ctrl->value = xctrl.value;
    + if (ret >= 0)
    + ctrl->value = xctrl.value;
    break;
    }

    @@ -634,7 +632,7 @@ static long uvc_v4l2_do_ioctl(struct fil
    ret = uvc_ctrl_set(chain, &xctrl);
    if (ret < 0) {
    uvc_ctrl_rollback(handle);
    - return ret == -ENOENT ? -EINVAL : ret;
    + return ret;
    }
    ret = uvc_ctrl_commit(handle, &xctrl, 1);
    if (ret == 0)
    @@ -661,7 +659,7 @@ static long uvc_v4l2_do_ioctl(struct fil
    uvc_ctrl_rollback(handle);
    ctrls->error_idx = ret == -ENOENT
    ? ctrls->count : i;
    - return ret == -ENOENT ? -EINVAL : ret;
    + return ret;
    }
    }
    ctrls->error_idx = 0;
    @@ -691,7 +689,7 @@ static long uvc_v4l2_do_ioctl(struct fil
    ctrls->error_idx = (ret == -ENOENT &&
    cmd == VIDIOC_S_EXT_CTRLS)
    ? ctrls->count : i;
    - return ret == -ENOENT ? -EINVAL : ret;
    + return ret;
    }
    }


    --
    I speak only for myself.
    Rafael J. Wysocki, Intel Open Source Technology Center.


    \
     
     \ /
      Last update: 2012-12-23 14:41    [W:0.028 / U:30.992 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site