Hi All, I have a situation where I have to lock the ioctl provided in my driver. I have a uni processor (ARM) system. I am using Mutex as the lock for my ioctl. DEFINE_MUTEX(&lock_ioctl); MyIoctl() { Mutex_lock(&lock_ioctl); Switch(){ ........... } Mutex_unlock(&lock_ioctl); return 0; } I just wanted to know am I using the best lock available for the situation aor do I have any flaw in my implementation??? And from LKD I read that "*lock the data not the code*" and which I am literally doing so ?? any comments on this ? I have interrupts in my driver which is nothing to do with the lock.I am taking care that where ever I return in my ioctl the lock is released. Thanks , Praveen
Hi Praveen, On Mon, Nov 14, 2011 at 11:22 PM, Praveen kumar <chatpravi@gmail.com> wrote:
Hi All, I have a situation where I have to lock the ioctl provided in my driver. I have a uni processor (ARM) system. I am using Mutex as the lock for my ioctl. DEFINE_MUTEX(&lock_ioctl); MyIoctl() { Mutex_lock(&lock_ioctl); Switch(){ ........... } Mutex_unlock(&lock_ioctl); return 0; } I just wanted to know am I using the best lock available for the situation aor do I have any flaw in my implementation???
And from LKD I read that "*lock the data not the code*" and which I am literally doing so ?? any comments on this ?
By creatiing something called lock_ioctl, I'd say you're locking the code. Locking the data means that lets suppose you have some data structure which requires mutual exclusion. Then I'd create a lock associated with that data structure (and probably the lock would be a member of the data structure), and anytime I needed to manipulate/access the data structure (in an atomic manner), I'd acquire the lock.
I have interrupts in my driver which is nothing to do with the lock.I am taking care that where ever I return in my ioctl the lock is released.
What are you locking? Does every single ioctl really need to acquire the lock? Why? -- Dave Hylands Shuswap, BC, Canada http://www.davehylands.com
The driver is opened by multiple processes, One such process is a diagnostics. Where I am checking sanity of some of the registers if I make ioctl open for all the processes.It can screw up my diagnostics check.(write some values to registers). This is basic reason I am using lock to my ioctl. Thanks Praveen On Tue, Nov 15, 2011 at 1:28 PM, Dave Hylands <dhylands@gmail.com> wrote:
Hi Praveen,
On Mon, Nov 14, 2011 at 11:22 PM, Praveen kumar <chatpravi@gmail.com> wrote:
Hi All, I have a situation where I have to lock the ioctl provided in my driver.
I
have a uni processor (ARM) system. I am using Mutex as the lock for my ioctl. DEFINE_MUTEX(&lock_ioctl); MyIoctl() { Mutex_lock(&lock_ioctl); Switch(){ ........... } Mutex_unlock(&lock_ioctl); return 0; } I just wanted to know am I using the best lock available for the situation aor do I have any flaw in my implementation???
And from LKD I read that "*lock the data not the code*" and which I am literally doing so ?? any comments on this ?
By creatiing something called lock_ioctl, I'd say you're locking the code.
Locking the data means that lets suppose you have some data structure which requires mutual exclusion. Then I'd create a lock associated with that data structure (and probably the lock would be a member of the data structure), and anytime I needed to manipulate/access the data structure (in an atomic manner), I'd acquire the lock.
I have interrupts in my driver which is nothing to do with the lock.I am taking care that where ever I return in my ioctl the lock is released.
What are you locking? Does every single ioctl really need to acquire the lock? Why?
-- Dave Hylands Shuswap, BC, Canada http://www.davehylands.com
Which kernel version are u using??? As far as I know, before 2.6.36, ioctl calls used to hold BKL, so no need to have separate locking mechanism. If you are using unlocked_ioctl (after 36 kernel) then you need to take care of locking. Now, what kind of lock you want to use, depends on your requirement. It could be code lock or data lock.. On Tue, Nov 15, 2011 at 5:53 PM, Praveen kumar <chatpravi@gmail.com> wrote:
The driver is opened by multiple processes, One such process is a diagnostics. Where I am checking sanity of some of the registers if I make ioctl open for all the processes.It can screw up my diagnostics check.(write some values to registers). This is basic reason I am using lock to my ioctl.
Thanks Praveen
On Tue, Nov 15, 2011 at 1:28 PM, Dave Hylands <dhylands@gmail.com> wrote:
Hi Praveen,
On Mon, Nov 14, 2011 at 11:22 PM, Praveen kumar <chatpravi@gmail.com> wrote:
Hi All, I have a situation where I have to lock the ioctl provided in my
driver. I
have a uni processor (ARM) system. I am using Mutex as the lock for my ioctl. DEFINE_MUTEX(&lock_ioctl); MyIoctl() { Mutex_lock(&lock_ioctl); Switch(){ ........... } Mutex_unlock(&lock_ioctl); return 0; } I just wanted to know am I using the best lock available for the situation aor do I have any flaw in my implementation???
And from LKD I read that "*lock the data not the code*" and which I am literally doing so ?? any comments on this ?
By creatiing something called lock_ioctl, I'd say you're locking the code.
Locking the data means that lets suppose you have some data structure which requires mutual exclusion. Then I'd create a lock associated with that data structure (and probably the lock would be a member of the data structure), and anytime I needed to manipulate/access the data structure (in an atomic manner), I'd acquire the lock.
I have interrupts in my driver which is nothing to do with the lock.I am taking care that where ever I return in my ioctl the lock is released.
What are you locking? Does every single ioctl really need to acquire the lock? Why?
-- Dave Hylands Shuswap, BC, Canada http://www.davehylands.com
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Hello, Testing the scullwuid produced attached crash logs. Combination of read and write using "dd" and "cat" caused this problem. I don't exactly remember exactly what caused the problem but combination of read and write did it. Reproduction would not be possible but just want to know why this happened as anyone looking into the code would come to the conclusion that proper locking is in place. Below is small part of the crash logs: [21028.412827] BUG: unable to handle kernel paging request at 00300d04 [21028.412832] IP: [<c035aef7>] _copy_from_user+0x97/0x130 [21028.412839] *pde = 6fa0b067 [21028.412842] Oops: 0003 [#1] SMP [21028.412933] Call Trace: [21028.412940] [<f9f24d84>] ? scull_write+0x193/0x204 [scull] [21028.412946] [<c0218fc2>] ? vfs_write+0xa2/0x190 [21028.412949] [<f9f24bf1>] ? scull_write+0x0/0x204 [scull] [21028.412953] [<c0219882>] ? sys_write+0x42/0x70 [21028.412958] [<c05cadd4>] ? syscall_call+0x7/0xb [21028.412963] [<c05c0000>] ? calibrate_delay_direct+0x5a/0xfb Some of the below code is removed for fitting it into this mail: ssize_t scull_write(struct file *filp, const char __user *buf, size_t count, loff_t *f_pos) { if (down_interruptible(&dev->sem)) return -ERESTARTSYS; dptr = scull_follow(dev, item); if (dptr == NULL) goto out; if (!dptr->data) { dptr->data = kmalloc(qset * sizeof(char *), GFP_KERNEL); if (!dptr->data) goto out; memset(dptr->data, 0, qset * sizeof(char *)); } if (!dptr->data[s_pos]) { dptr->data[s_pos] = kmalloc(quantum, GFP_KERNEL); if (!dptr->data[s_pos]) goto out; /* write only up to the end of this quantum */ if (count > quantum - q_pos) count = quantum - q_pos; if (copy_from_user(dptr->data[s_pos]+q_pos, buf, count)) { retval = -EFAULT; goto out; } Used: https://github.com/martinezjavier/ldd3 box:Ubuntu 10.10 |2.6.35-27-generic #48-Ubuntu SMP i686 GNU/Linux --thanks
On Sat, Nov 19, 2011 at 1:34 PM, anish kumar <anish198519851985@gmail.com> wrote:
Hello,
Testing the scullwuid produced attached crash logs.
Hello Anish, Sadly I don't have more time to keep updates the LDD3 examples. I did a long time ago to use for academic purposes. As you can see my last commit is from almost a year ago and even then I only used to be sure that the modules compiles cleanly without warning (not sure if they compile anymore, probably not) but didn't try it, since I don't use the modules anymore. Hope that someone with more time than me can fork the git tree and continue with this work since the LDD3 book is an amazing book to introduce newcomers to Linux driver development. Best regards, -- Javier Martínez Canillas (+34) 682 39 81 69 Barcelona, Spain
Hi Praveen, On Tue, Nov 15, 2011 at 4:23 AM, Praveen kumar <chatpravi@gmail.com> wrote:
The driver is opened by multiple processes, One such process is a diagnostics. Where I am checking sanity of some of the registers if I make ioctl open for all the processes.It can screw up my diagnostics check.(write some values to registers). This is basic reason I am using lock to my ioctl.
So "locking the data" means that you should create a lock for the data registers in question. You should acquire the lock around writing the values to the registers, and presumably around reading them. Acquiring the lock is about accessing the registers, not calling the ioctl, even if today the only place that you access the registers is from the ioctl. What happens if down the road you decide to add some type of monitoring thread and it needs to access the registers? It's not in an ioctl, so acquiring some type of ioctl lock is misleading. The data that the lock is tied to is the registers, so make the code work that way. Also, som paths through the ioctl might not need to access the registers, so you don't need or want to acquire the lock for those cases. -- Dave Hylands Shuswap, BC, Canada http://www.davehylands.com
participants (5)
-
anish kumar -
Dave Hylands -
Javier Martinez Canillas -
Praveen kumar -
Tirtha Ghosh