Commit 30ad64b8 authored by Nikolaus Schulz's avatar Nikolaus Schulz Committed by Mauro Carvalho Chehab

[media] dvb: push down ioctl lock in dvb_usercopy

Since most dvb ioctls wrap their real work with dvb_usercopy, the static mutex
used in dvb_usercopy effectively is a global lock for dvb ioctls.
Unfortunately, frontend ioctls can be blocked by the frontend thread for
several seconds; this leads to unacceptable lock contention.  Mitigate that by
pushing the mutex from dvb_usercopy down to the individual, device specific
ioctls.
There are 10 such ioctl functions using dvb_usercopy, either calling it
directly, or via the trivial wrapper dvb_generic_ioctl. The following already
employ their own locking and look safe:
    • dvb_demux_ioctl           (as per dvb_demux_do_ioctl)
    • dvb_dvr_ioctl             (as per dvb_dvr_do_ioctl)
    • dvb_osd_ioctl             (as per single non-trivial callee)
    • fdtv_ca_ioctl             (as per callees)
    • dvb_frontend_ioctl
The following functions do not, and are thus changed to use a device specific
mutex:
    • dvb_net_ioctl             (as per dvb_net_do_ioctl)
    • dvb_ca_en50221_io_ioctl   (as per dvb_ca_en50221_io_do_ioctl)
    • dvb_video_ioctl
    • dvb_audio_ioctl
    • dvb_ca_ioctl
Signed-off-by: default avatarNikolaus Schulz <schulz@macnetix.de>
Signed-off-by: default avatarMichael Krufky <mkrufky@linuxtv.org>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@redhat.com>
parent 6ae23224
...@@ -156,6 +156,9 @@ struct dvb_ca_private { ...@@ -156,6 +156,9 @@ struct dvb_ca_private {
/* Slot to start looking for data to read from in the next user-space read operation */ /* Slot to start looking for data to read from in the next user-space read operation */
int next_read_slot; int next_read_slot;
/* mutex serializing ioctls */
struct mutex ioctl_mutex;
}; };
static void dvb_ca_en50221_thread_wakeup(struct dvb_ca_private *ca); static void dvb_ca_en50221_thread_wakeup(struct dvb_ca_private *ca);
...@@ -1191,6 +1194,9 @@ static int dvb_ca_en50221_io_do_ioctl(struct file *file, ...@@ -1191,6 +1194,9 @@ static int dvb_ca_en50221_io_do_ioctl(struct file *file,
dprintk("%s\n", __func__); dprintk("%s\n", __func__);
if (mutex_lock_interruptible(&ca->ioctl_mutex))
return -ERESTARTSYS;
switch (cmd) { switch (cmd) {
case CA_RESET: case CA_RESET:
for (slot = 0; slot < ca->slot_count; slot++) { for (slot = 0; slot < ca->slot_count; slot++) {
...@@ -1241,6 +1247,7 @@ static int dvb_ca_en50221_io_do_ioctl(struct file *file, ...@@ -1241,6 +1247,7 @@ static int dvb_ca_en50221_io_do_ioctl(struct file *file,
break; break;
} }
mutex_unlock(&ca->ioctl_mutex);
return err; return err;
} }
...@@ -1695,6 +1702,8 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter, ...@@ -1695,6 +1702,8 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
mutex_init(&ca->slot_info[i].slot_lock); mutex_init(&ca->slot_info[i].slot_lock);
} }
mutex_init(&ca->ioctl_mutex);
if (signal_pending(current)) { if (signal_pending(current)) {
ret = -EINTR; ret = -EINTR;
goto error; goto error;
......
...@@ -1345,26 +1345,35 @@ static int dvb_net_do_ioctl(struct file *file, ...@@ -1345,26 +1345,35 @@ static int dvb_net_do_ioctl(struct file *file,
{ {
struct dvb_device *dvbdev = file->private_data; struct dvb_device *dvbdev = file->private_data;
struct dvb_net *dvbnet = dvbdev->priv; struct dvb_net *dvbnet = dvbdev->priv;
int ret = 0;
if (((file->f_flags&O_ACCMODE)==O_RDONLY)) if (((file->f_flags&O_ACCMODE)==O_RDONLY))
return -EPERM; return -EPERM;
if (mutex_lock_interruptible(&dvbnet->ioctl_mutex))
return -ERESTARTSYS;
switch (cmd) { switch (cmd) {
case NET_ADD_IF: case NET_ADD_IF:
{ {
struct dvb_net_if *dvbnetif = parg; struct dvb_net_if *dvbnetif = parg;
int result; int result;
if (!capable(CAP_SYS_ADMIN)) if (!capable(CAP_SYS_ADMIN)) {
return -EPERM; ret = -EPERM;
goto ioctl_error;
}
if (!try_module_get(dvbdev->adapter->module)) if (!try_module_get(dvbdev->adapter->module)) {
return -EPERM; ret = -EPERM;
goto ioctl_error;
}
result=dvb_net_add_if(dvbnet, dvbnetif->pid, dvbnetif->feedtype); result=dvb_net_add_if(dvbnet, dvbnetif->pid, dvbnetif->feedtype);
if (result<0) { if (result<0) {
module_put(dvbdev->adapter->module); module_put(dvbdev->adapter->module);
return result; ret = result;
goto ioctl_error;
} }
dvbnetif->if_num=result; dvbnetif->if_num=result;
break; break;
...@@ -1376,8 +1385,10 @@ static int dvb_net_do_ioctl(struct file *file, ...@@ -1376,8 +1385,10 @@ static int dvb_net_do_ioctl(struct file *file,
struct dvb_net_if *dvbnetif = parg; struct dvb_net_if *dvbnetif = parg;
if (dvbnetif->if_num >= DVB_NET_DEVICES_MAX || if (dvbnetif->if_num >= DVB_NET_DEVICES_MAX ||
!dvbnet->state[dvbnetif->if_num]) !dvbnet->state[dvbnetif->if_num]) {
return -EINVAL; ret = -EINVAL;
goto ioctl_error;
}
netdev = dvbnet->device[dvbnetif->if_num]; netdev = dvbnet->device[dvbnetif->if_num];
...@@ -1388,16 +1399,18 @@ static int dvb_net_do_ioctl(struct file *file, ...@@ -1388,16 +1399,18 @@ static int dvb_net_do_ioctl(struct file *file,
} }
case NET_REMOVE_IF: case NET_REMOVE_IF:
{ {
int ret; if (!capable(CAP_SYS_ADMIN)) {
ret = -EPERM;
if (!capable(CAP_SYS_ADMIN)) goto ioctl_error;
return -EPERM; }
if ((unsigned long) parg >= DVB_NET_DEVICES_MAX) if ((unsigned long) parg >= DVB_NET_DEVICES_MAX) {
return -EINVAL; ret = -EINVAL;
goto ioctl_error;
}
ret = dvb_net_remove_if(dvbnet, (unsigned long) parg); ret = dvb_net_remove_if(dvbnet, (unsigned long) parg);
if (!ret) if (!ret)
module_put(dvbdev->adapter->module); module_put(dvbdev->adapter->module);
return ret; break;
} }
/* binary compatibility cruft */ /* binary compatibility cruft */
...@@ -1406,16 +1419,21 @@ static int dvb_net_do_ioctl(struct file *file, ...@@ -1406,16 +1419,21 @@ static int dvb_net_do_ioctl(struct file *file,
struct __dvb_net_if_old *dvbnetif = parg; struct __dvb_net_if_old *dvbnetif = parg;
int result; int result;
if (!capable(CAP_SYS_ADMIN)) if (!capable(CAP_SYS_ADMIN)) {
return -EPERM; ret = -EPERM;
goto ioctl_error;
}
if (!try_module_get(dvbdev->adapter->module)) if (!try_module_get(dvbdev->adapter->module)) {
return -EPERM; ret = -EPERM;
goto ioctl_error;
}
result=dvb_net_add_if(dvbnet, dvbnetif->pid, DVB_NET_FEEDTYPE_MPE); result=dvb_net_add_if(dvbnet, dvbnetif->pid, DVB_NET_FEEDTYPE_MPE);
if (result<0) { if (result<0) {
module_put(dvbdev->adapter->module); module_put(dvbdev->adapter->module);
return result; ret = result;
goto ioctl_error;
} }
dvbnetif->if_num=result; dvbnetif->if_num=result;
break; break;
...@@ -1427,8 +1445,10 @@ static int dvb_net_do_ioctl(struct file *file, ...@@ -1427,8 +1445,10 @@ static int dvb_net_do_ioctl(struct file *file,
struct __dvb_net_if_old *dvbnetif = parg; struct __dvb_net_if_old *dvbnetif = parg;
if (dvbnetif->if_num >= DVB_NET_DEVICES_MAX || if (dvbnetif->if_num >= DVB_NET_DEVICES_MAX ||
!dvbnet->state[dvbnetif->if_num]) !dvbnet->state[dvbnetif->if_num]) {
return -EINVAL; ret = -EINVAL;
goto ioctl_error;
}
netdev = dvbnet->device[dvbnetif->if_num]; netdev = dvbnet->device[dvbnetif->if_num];
...@@ -1437,9 +1457,13 @@ static int dvb_net_do_ioctl(struct file *file, ...@@ -1437,9 +1457,13 @@ static int dvb_net_do_ioctl(struct file *file,
break; break;
} }
default: default:
return -ENOTTY; ret = -ENOTTY;
break;
} }
return 0;
ioctl_error:
mutex_unlock(&dvbnet->ioctl_mutex);
return ret;
} }
static long dvb_net_ioctl(struct file *file, static long dvb_net_ioctl(struct file *file,
...@@ -1505,6 +1529,7 @@ int dvb_net_init (struct dvb_adapter *adap, struct dvb_net *dvbnet, ...@@ -1505,6 +1529,7 @@ int dvb_net_init (struct dvb_adapter *adap, struct dvb_net *dvbnet,
{ {
int i; int i;
mutex_init(&dvbnet->ioctl_mutex);
dvbnet->demux = dmx; dvbnet->demux = dmx;
for (i=0; i<DVB_NET_DEVICES_MAX; i++) for (i=0; i<DVB_NET_DEVICES_MAX; i++)
......
...@@ -40,6 +40,7 @@ struct dvb_net { ...@@ -40,6 +40,7 @@ struct dvb_net {
int state[DVB_NET_DEVICES_MAX]; int state[DVB_NET_DEVICES_MAX];
unsigned int exit:1; unsigned int exit:1;
struct dmx_demux *demux; struct dmx_demux *demux;
struct mutex ioctl_mutex;
}; };
void dvb_net_release(struct dvb_net *); void dvb_net_release(struct dvb_net *);
......
...@@ -418,10 +418,8 @@ int dvb_usercopy(struct file *file, ...@@ -418,10 +418,8 @@ int dvb_usercopy(struct file *file,
} }
/* call driver */ /* call driver */
mutex_lock(&dvbdev_mutex);
if ((err = func(file, cmd, parg)) == -ENOIOCTLCMD) if ((err = func(file, cmd, parg)) == -ENOIOCTLCMD)
err = -ENOTTY; err = -ENOTTY;
mutex_unlock(&dvbdev_mutex);
if (err < 0) if (err < 0)
goto out; goto out;
......
...@@ -2723,6 +2723,8 @@ static int __devinit av7110_attach(struct saa7146_dev* dev, ...@@ -2723,6 +2723,8 @@ static int __devinit av7110_attach(struct saa7146_dev* dev,
if (ret < 0) if (ret < 0)
goto err_av7110_exit_v4l_12; goto err_av7110_exit_v4l_12;
mutex_init(&av7110->ioctl_mutex);
#if defined(CONFIG_INPUT_EVDEV) || defined(CONFIG_INPUT_EVDEV_MODULE) #if defined(CONFIG_INPUT_EVDEV) || defined(CONFIG_INPUT_EVDEV_MODULE)
av7110_ir_init(av7110); av7110_ir_init(av7110);
#endif #endif
......
...@@ -271,6 +271,8 @@ struct av7110 { ...@@ -271,6 +271,8 @@ struct av7110 {
struct dvb_frontend* fe; struct dvb_frontend* fe;
fe_status_t fe_status; fe_status_t fe_status;
struct mutex ioctl_mutex;
/* crash recovery */ /* crash recovery */
void (*recover)(struct av7110* av7110); void (*recover)(struct av7110* av7110);
fe_sec_voltage_t saved_voltage; fe_sec_voltage_t saved_voltage;
......
...@@ -1109,6 +1109,9 @@ static int dvb_video_ioctl(struct file *file, ...@@ -1109,6 +1109,9 @@ static int dvb_video_ioctl(struct file *file,
} }
} }
if (mutex_lock_interruptible(&av7110->ioctl_mutex))
return -ERESTARTSYS;
switch (cmd) { switch (cmd) {
case VIDEO_STOP: case VIDEO_STOP:
av7110->videostate.play_state = VIDEO_STOPPED; av7110->videostate.play_state = VIDEO_STOPPED;
...@@ -1297,6 +1300,7 @@ static int dvb_video_ioctl(struct file *file, ...@@ -1297,6 +1300,7 @@ static int dvb_video_ioctl(struct file *file,
break; break;
} }
mutex_unlock(&av7110->ioctl_mutex);
return ret; return ret;
} }
...@@ -1314,6 +1318,9 @@ static int dvb_audio_ioctl(struct file *file, ...@@ -1314,6 +1318,9 @@ static int dvb_audio_ioctl(struct file *file,
(cmd != AUDIO_GET_STATUS)) (cmd != AUDIO_GET_STATUS))
return -EPERM; return -EPERM;
if (mutex_lock_interruptible(&av7110->ioctl_mutex))
return -ERESTARTSYS;
switch (cmd) { switch (cmd) {
case AUDIO_STOP: case AUDIO_STOP:
if (av7110->audiostate.stream_source == AUDIO_SOURCE_MEMORY) if (av7110->audiostate.stream_source == AUDIO_SOURCE_MEMORY)
...@@ -1442,6 +1449,7 @@ static int dvb_audio_ioctl(struct file *file, ...@@ -1442,6 +1449,7 @@ static int dvb_audio_ioctl(struct file *file,
ret = -ENOIOCTLCMD; ret = -ENOIOCTLCMD;
} }
mutex_unlock(&av7110->ioctl_mutex);
return ret; return ret;
} }
......
...@@ -253,12 +253,17 @@ static int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg) ...@@ -253,12 +253,17 @@ static int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg)
struct dvb_device *dvbdev = file->private_data; struct dvb_device *dvbdev = file->private_data;
struct av7110 *av7110 = dvbdev->priv; struct av7110 *av7110 = dvbdev->priv;
unsigned long arg = (unsigned long) parg; unsigned long arg = (unsigned long) parg;
int ret = 0;
dprintk(8, "av7110:%p\n",av7110); dprintk(8, "av7110:%p\n",av7110);
if (mutex_lock_interruptible(&av7110->ioctl_mutex))
return -ERESTARTSYS;
switch (cmd) { switch (cmd) {
case CA_RESET: case CA_RESET:
return ci_ll_reset(&av7110->ci_wbuffer, file, arg, &av7110->ci_slot[0]); ret = ci_ll_reset(&av7110->ci_wbuffer, file, arg,
&av7110->ci_slot[0]);
break; break;
case CA_GET_CAP: case CA_GET_CAP:
{ {
...@@ -277,8 +282,10 @@ static int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg) ...@@ -277,8 +282,10 @@ static int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg)
{ {
ca_slot_info_t *info=(ca_slot_info_t *)parg; ca_slot_info_t *info=(ca_slot_info_t *)parg;
if (info->num < 0 || info->num > 1) if (info->num < 0 || info->num > 1) {
mutex_unlock(&av7110->ioctl_mutex);
return -EINVAL; return -EINVAL;
}
av7110->ci_slot[info->num].num = info->num; av7110->ci_slot[info->num].num = info->num;
av7110->ci_slot[info->num].type = FW_CI_LL_SUPPORT(av7110->arm_app) ? av7110->ci_slot[info->num].type = FW_CI_LL_SUPPORT(av7110->arm_app) ?
CA_CI_LINK : CA_CI; CA_CI_LINK : CA_CI;
...@@ -306,10 +313,10 @@ static int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg) ...@@ -306,10 +313,10 @@ static int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg)
{ {
ca_descr_t *descr = (ca_descr_t*) parg; ca_descr_t *descr = (ca_descr_t*) parg;
if (descr->index >= 16) if (descr->index >= 16 || descr->parity > 1) {
return -EINVAL; mutex_unlock(&av7110->ioctl_mutex);
if (descr->parity > 1)
return -EINVAL; return -EINVAL;
}
av7110_fw_cmd(av7110, COMTYPE_PIDFILTER, SetDescr, 5, av7110_fw_cmd(av7110, COMTYPE_PIDFILTER, SetDescr, 5,
(descr->index<<8)|descr->parity, (descr->index<<8)|descr->parity,
(descr->cw[0]<<8)|descr->cw[1], (descr->cw[0]<<8)|descr->cw[1],
...@@ -320,9 +327,12 @@ static int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg) ...@@ -320,9 +327,12 @@ static int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg)
} }
default: default:
return -EINVAL; ret = -EINVAL;
break;
} }
return 0;
mutex_unlock(&av7110->ioctl_mutex);
return ret;
} }
static ssize_t dvb_ca_write(struct file *file, const char __user *buf, static ssize_t dvb_ca_write(struct file *file, const char __user *buf,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment