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    [from the cache]
©2003-2011 Jasper Spaans