From 30ad64b8ac539459f8975aa186421ef3db0bb5cb Mon Sep 17 00:00:00 2001 From: Nikolaus Schulz Date: Sun, 23 Dec 2012 18:49:07 -0300 Subject: [PATCH] [media] dvb: push down ioctl lock in dvb_usercopy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: Nikolaus Schulz Signed-off-by: Michael Krufky Signed-off-by: Mauro Carvalho Chehab --- drivers/media/dvb-core/dvb_ca_en50221.c | 9 ++++ drivers/media/dvb-core/dvb_net.c | 71 +++++++++++++++++-------- drivers/media/dvb-core/dvb_net.h | 1 + drivers/media/dvb-core/dvbdev.c | 2 - drivers/media/pci/ttpci/av7110.c | 2 + drivers/media/pci/ttpci/av7110.h | 2 + drivers/media/pci/ttpci/av7110_av.c | 8 +++ drivers/media/pci/ttpci/av7110_ca.c | 24 ++++++--- 8 files changed, 87 insertions(+), 32 deletions(-) diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c index 9be65a3b931f..190e5e0f48c7 100644 --- a/drivers/media/dvb-core/dvb_ca_en50221.c +++ b/drivers/media/dvb-core/dvb_ca_en50221.c @@ -156,6 +156,9 @@ struct dvb_ca_private { /* Slot to start looking for data to read from in the next user-space read operation */ int next_read_slot; + + /* mutex serializing ioctls */ + struct mutex ioctl_mutex; }; 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, dprintk("%s\n", __func__); + if (mutex_lock_interruptible(&ca->ioctl_mutex)) + return -ERESTARTSYS; + switch (cmd) { case CA_RESET: for (slot = 0; slot < ca->slot_count; slot++) { @@ -1241,6 +1247,7 @@ static int dvb_ca_en50221_io_do_ioctl(struct file *file, break; } + mutex_unlock(&ca->ioctl_mutex); return err; } @@ -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->ioctl_mutex); + if (signal_pending(current)) { ret = -EINTR; goto error; diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c index c2117688aa23..44225b186f6d 100644 --- a/drivers/media/dvb-core/dvb_net.c +++ b/drivers/media/dvb-core/dvb_net.c @@ -1345,26 +1345,35 @@ static int dvb_net_do_ioctl(struct file *file, { struct dvb_device *dvbdev = file->private_data; struct dvb_net *dvbnet = dvbdev->priv; + int ret = 0; if (((file->f_flags&O_ACCMODE)==O_RDONLY)) return -EPERM; + if (mutex_lock_interruptible(&dvbnet->ioctl_mutex)) + return -ERESTARTSYS; + switch (cmd) { case NET_ADD_IF: { struct dvb_net_if *dvbnetif = parg; int result; - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; + if (!capable(CAP_SYS_ADMIN)) { + ret = -EPERM; + goto ioctl_error; + } - if (!try_module_get(dvbdev->adapter->module)) - return -EPERM; + if (!try_module_get(dvbdev->adapter->module)) { + ret = -EPERM; + goto ioctl_error; + } result=dvb_net_add_if(dvbnet, dvbnetif->pid, dvbnetif->feedtype); if (result<0) { module_put(dvbdev->adapter->module); - return result; + ret = result; + goto ioctl_error; } dvbnetif->if_num=result; break; @@ -1376,8 +1385,10 @@ static int dvb_net_do_ioctl(struct file *file, struct dvb_net_if *dvbnetif = parg; if (dvbnetif->if_num >= DVB_NET_DEVICES_MAX || - !dvbnet->state[dvbnetif->if_num]) - return -EINVAL; + !dvbnet->state[dvbnetif->if_num]) { + ret = -EINVAL; + goto ioctl_error; + } netdev = dvbnet->device[dvbnetif->if_num]; @@ -1388,16 +1399,18 @@ static int dvb_net_do_ioctl(struct file *file, } case NET_REMOVE_IF: { - int ret; - - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; - if ((unsigned long) parg >= DVB_NET_DEVICES_MAX) - return -EINVAL; + if (!capable(CAP_SYS_ADMIN)) { + ret = -EPERM; + goto ioctl_error; + } + if ((unsigned long) parg >= DVB_NET_DEVICES_MAX) { + ret = -EINVAL; + goto ioctl_error; + } ret = dvb_net_remove_if(dvbnet, (unsigned long) parg); if (!ret) module_put(dvbdev->adapter->module); - return ret; + break; } /* binary compatibility cruft */ @@ -1406,16 +1419,21 @@ static int dvb_net_do_ioctl(struct file *file, struct __dvb_net_if_old *dvbnetif = parg; int result; - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; + if (!capable(CAP_SYS_ADMIN)) { + ret = -EPERM; + goto ioctl_error; + } - if (!try_module_get(dvbdev->adapter->module)) - return -EPERM; + if (!try_module_get(dvbdev->adapter->module)) { + ret = -EPERM; + goto ioctl_error; + } result=dvb_net_add_if(dvbnet, dvbnetif->pid, DVB_NET_FEEDTYPE_MPE); if (result<0) { module_put(dvbdev->adapter->module); - return result; + ret = result; + goto ioctl_error; } dvbnetif->if_num=result; break; @@ -1427,8 +1445,10 @@ static int dvb_net_do_ioctl(struct file *file, struct __dvb_net_if_old *dvbnetif = parg; if (dvbnetif->if_num >= DVB_NET_DEVICES_MAX || - !dvbnet->state[dvbnetif->if_num]) - return -EINVAL; + !dvbnet->state[dvbnetif->if_num]) { + ret = -EINVAL; + goto ioctl_error; + } netdev = dvbnet->device[dvbnetif->if_num]; @@ -1437,9 +1457,13 @@ static int dvb_net_do_ioctl(struct file *file, break; } 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, @@ -1505,6 +1529,7 @@ int dvb_net_init (struct dvb_adapter *adap, struct dvb_net *dvbnet, { int i; + mutex_init(&dvbnet->ioctl_mutex); dvbnet->demux = dmx; for (i=0; iioctl_mutex); + #if defined(CONFIG_INPUT_EVDEV) || defined(CONFIG_INPUT_EVDEV_MODULE) av7110_ir_init(av7110); #endif diff --git a/drivers/media/pci/ttpci/av7110.h b/drivers/media/pci/ttpci/av7110.h index a378662b1dcf..ef3d9606b269 100644 --- a/drivers/media/pci/ttpci/av7110.h +++ b/drivers/media/pci/ttpci/av7110.h @@ -271,6 +271,8 @@ struct av7110 { struct dvb_frontend* fe; fe_status_t fe_status; + struct mutex ioctl_mutex; + /* crash recovery */ void (*recover)(struct av7110* av7110); fe_sec_voltage_t saved_voltage; diff --git a/drivers/media/pci/ttpci/av7110_av.c b/drivers/media/pci/ttpci/av7110_av.c index 952b33dbac4f..301029ca4535 100644 --- a/drivers/media/pci/ttpci/av7110_av.c +++ b/drivers/media/pci/ttpci/av7110_av.c @@ -1109,6 +1109,9 @@ static int dvb_video_ioctl(struct file *file, } } + if (mutex_lock_interruptible(&av7110->ioctl_mutex)) + return -ERESTARTSYS; + switch (cmd) { case VIDEO_STOP: av7110->videostate.play_state = VIDEO_STOPPED; @@ -1297,6 +1300,7 @@ static int dvb_video_ioctl(struct file *file, break; } + mutex_unlock(&av7110->ioctl_mutex); return ret; } @@ -1314,6 +1318,9 @@ static int dvb_audio_ioctl(struct file *file, (cmd != AUDIO_GET_STATUS)) return -EPERM; + if (mutex_lock_interruptible(&av7110->ioctl_mutex)) + return -ERESTARTSYS; + switch (cmd) { case AUDIO_STOP: if (av7110->audiostate.stream_source == AUDIO_SOURCE_MEMORY) @@ -1442,6 +1449,7 @@ static int dvb_audio_ioctl(struct file *file, ret = -ENOIOCTLCMD; } + mutex_unlock(&av7110->ioctl_mutex); return ret; } diff --git a/drivers/media/pci/ttpci/av7110_ca.c b/drivers/media/pci/ttpci/av7110_ca.c index 9fc1dd0ba4c3..a6079b90252a 100644 --- a/drivers/media/pci/ttpci/av7110_ca.c +++ b/drivers/media/pci/ttpci/av7110_ca.c @@ -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 av7110 *av7110 = dvbdev->priv; unsigned long arg = (unsigned long) parg; + int ret = 0; dprintk(8, "av7110:%p\n",av7110); + if (mutex_lock_interruptible(&av7110->ioctl_mutex)) + return -ERESTARTSYS; + switch (cmd) { 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; case CA_GET_CAP: { @@ -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; - if (info->num < 0 || info->num > 1) + if (info->num < 0 || info->num > 1) { + mutex_unlock(&av7110->ioctl_mutex); return -EINVAL; + } av7110->ci_slot[info->num].num = info->num; av7110->ci_slot[info->num].type = FW_CI_LL_SUPPORT(av7110->arm_app) ? CA_CI_LINK : CA_CI; @@ -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; - if (descr->index >= 16) - return -EINVAL; - if (descr->parity > 1) + if (descr->index >= 16 || descr->parity > 1) { + mutex_unlock(&av7110->ioctl_mutex); return -EINVAL; + } av7110_fw_cmd(av7110, COMTYPE_PIDFILTER, SetDescr, 5, (descr->index<<8)|descr->parity, (descr->cw[0]<<8)|descr->cw[1], @@ -320,9 +327,12 @@ static int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg) } 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,