2009-03-24 21:17:44

by Stoyan Gaydarov

Subject: [PATCH 12/13] [drivers/media] changed ioctls to unlocked

From: Stoyan Gaydarov <[email protected]>

Signed-off-by: Stoyan Gaydarov <[email protected]>
---
drivers/media/dvb/bt8xx/dst_ca.c | 7 +++++--
drivers/media/video/dabusb.c | 11 ++++++++---
2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/media/dvb/bt8xx/dst_ca.c b/drivers/media/dvb/bt8xx/dst_ca.c
index 0258451..d3487c5 100644
--- a/drivers/media/dvb/bt8xx/dst_ca.c
+++ b/drivers/media/dvb/bt8xx/dst_ca.c
@@ -552,8 +552,10 @@ free_mem_and_exit:
return result;
}

-static int dst_ca_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long ioctl_arg)
+static long dst_ca_ioctl(struct file *file, unsigned int cmd, unsigned long ioctl_arg)
{
+ lock_kernel();
+
struct dvb_device* dvbdev = (struct dvb_device*) file->private_data;
struct dst_state* state = (struct dst_state*) dvbdev->priv;
struct ca_slot_info *p_ca_slot_info;
@@ -647,6 +649,7 @@ static int dst_ca_ioctl(struct inode *inode, struct file *file, unsigned int cmd
kfree (p_ca_slot_info);
kfree (p_ca_caps);

+ unlock_kernel();
return result;
}

@@ -684,7 +687,7 @@ static ssize_t dst_ca_write(struct file *file, const char __user *buffer, size_t

static struct file_operations dst_ca_fops = {
.owner = THIS_MODULE,
- .ioctl = dst_ca_ioctl,
+ .unlocked_ioctl = dst_ca_ioctl,
.open = dst_ca_open,
.release = dst_ca_release,
.read = dst_ca_read,
diff --git a/drivers/media/video/dabusb.c b/drivers/media/video/dabusb.c
index 298810d..ae4cd79 100644
--- a/drivers/media/video/dabusb.c
+++ b/drivers/media/video/dabusb.c
@@ -657,8 +657,9 @@ static int dabusb_release (struct inode *inode, struct file *file)
return 0;
}

-static int dabusb_ioctl (struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
+static long dabusb_ioctl (struct file *file, unsigned int cmd, unsigned long arg)
{
+ lock_kernel();
pdabusb_t s = (pdabusb_t) file->private_data;
pbulk_transfer_t pbulk;
int ret = 0;
@@ -666,13 +667,16 @@ static int dabusb_ioctl (struct inode *inode, struct file *file, unsigned int cm

dbg("dabusb_ioctl");

- if (s->remove_pending)
+ if (s->remove_pending) {
+ unlock_kernel();
return -EIO;
+ }

mutex_lock(&s->mutex);

if (!s->usbdev) {
mutex_unlock(&s->mutex);
+ unlock_kernel();
return -EIO;
}

@@ -713,6 +717,7 @@ static int dabusb_ioctl (struct inode *inode, struct file *file, unsigned int cm
break;
}
mutex_unlock(&s->mutex);
+ unlock_kernel();
return ret;
}

@@ -721,7 +726,7 @@ static const struct file_operations dabusb_fops =
.owner = THIS_MODULE,
.llseek = no_llseek,
.read = dabusb_read,
- .ioctl = dabusb_ioctl,
+ .unlocked_ioctl = dabusb_ioctl,
.open = dabusb_open,
.release = dabusb_release,
};
--
1.6.2

2009-03-24 21:23:58

by Alan

Subject: Re: [PATCH 12/13] [drivers/media] changed ioctls to unlocked

> -static int dabusb_ioctl (struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
> +static long dabusb_ioctl (struct file *file, unsigned int cmd, unsigned long arg)
> {
> + lock_kernel();
> pdabusb_t s = (pdabusb_t) file->private_data;

After the variables or you'll get lots of warnings from gcc

2009-03-24 21:32:24

by Stoyan Gaydarov

Subject: Re: [PATCH 12/13] [drivers/media] changed ioctls to unlocked

On Tue, Mar 24, 2009 at 4:24 PM, Alan Cox <[email protected]> wrote:
>
>> -static int dabusb_ioctl (struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
>> +static long dabusb_ioctl (struct file *file, unsigned int cmd, unsigned long arg)
>>  {
>> +     lock_kernel();
>>       pdabusb_t s = (pdabusb_t) file->private_data;
>
> After the variables or you'll get lots of warnings from gcc
>
>

Unfortunately I am not familiar with this driver and as such i was not
sure if the variable required the lock to be accessed or not so as to
play it safe i put it before the variable. But i can resubmit this
patch if there are no problems.

--

-Stoyan

2009-03-24 21:39:52

by Sam Ravnborg

Subject: Re: [PATCH 12/13] [drivers/media] changed ioctls to unlocked

On Tue, Mar 24, 2009 at 04:31:54PM -0500, Stoyan Gaydarov wrote:
> On Tue, Mar 24, 2009 at 4:24 PM, Alan Cox <[email protected]> wrote:
> >
> >> -static int dabusb_ioctl (struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
> >> +static long dabusb_ioctl (struct file *file, unsigned int cmd, unsigned long arg)
> >> ?{
> >> + ? ? lock_kernel();
> >> ? ? ? pdabusb_t s = (pdabusb_t) file->private_data;
> >
> > After the variables or you'll get lots of warnings from gcc
> >
> >
>
> Unfortunately I am not familiar with this driver and as such i was not
> sure if the variable required the lock to be accessed or not so as to
> play it safe i put it before the variable. But i can resubmit this
> patch if there are no problems.

Please do so.

It is considered better style to first decalre the variable and then later
assign it.
So this would allow you to move the assignment after the lock_kernel(),

Sam