Skip to contentSkip to conversation

tty_ioctl: locking for tty_wait_until_sent

PATCH In review Alan Cox 3 participants 5 messages Mar 10, 2008 Char and misc drivers

lore
Share & tools
lore

Alan Cox

author
#0 lore raw

PATCH 1/1 tty_ioctl: locking for tty_wait_until_sent

This function still depends on the big kernel lock in some cases. Push locking into the function ready for removal of the BKL from ioctl call paths.

Signed-off-by Alan Cox <[email protected]>
drivers/char/tty_ioctl.c +7 -4
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.25-rc3-mm1/drivers/char/tty_ioctl.c linux-2.6.25-rc3-mm1/drivers/char/tty_ioctl.c
@@ -21,6 +21,7 @@
 #include <linux/module.h>
 #include <linux/bitops.h>
 #include <linux/mutex.h>
+#include <linux/smp_lock.h>
 
 #include <asm/io.h>
 #include <asm/uaccess.h>

@@ -61,11 +62,13 @@
 		return;
 	if (!timeout)
 		timeout = MAX_SCHEDULE_TIMEOUT;
+	lock_kernel();
 	if (wait_event_interruptible_timeout(tty->write_wait,
-			!tty->driver->chars_in_buffer(tty), timeout) < 0)
-		return;
-	if (tty->driver->wait_until_sent)
-		tty->driver->wait_until_sent(tty, timeout);
+			!tty->driver->chars_in_buffer(tty), timeout) >= 0) {
+		if (tty->driver->wait_until_sent)
+			tty->driver->wait_until_sent(tty, timeout);
+	}
+	unlock_kernel();
 }
 EXPORT_SYMBOL(tty_wait_until_sent);
#1 lore raw
Quoted 3 lines · Alan Cox

This function still depends on the big kernel lock in some cases. Push locking into the function ready for removal of the BKL from ioctl call paths.

Didn't you forget the .ioctl -> .unlocked_ioctl change?

-Andi

author
#2 lore raw
Quoted 7 lines · On 10 Mar 2008 23:12:51 +0100 Andi Kleen <[email protected]> wrote:
Quoted 3 lines · Alan Cox

This function still depends on the big kernel lock in some cases. Push locking into the function ready for removal of the BKL from ioctl call paths.

Didn't you forget the .ioctl -> .unlocked_ioctl change?

We are not yet ready to unlock the device ioctl paths for tty. We still explicitly take the BKL in the ioctl paths when calling the following methods

driver:

	->wait_until_sent()
	->break_ctl()
	->tiocmget
	->tiocmset
	->ioctl
ldisc:
	->ioctl

As well as all the open/close/hangup/ldisc change logic

I'm pretty close to removing it from the modem , ioctl and break methods and its working for me but needs a few drivers tweaking further.

#3 lore raw
Quoted 5 lines · Alan Cox
Quoted 2 lines · On 10 Mar 2008 23:12:51 +0100 Andi Kleen <[email protected]> wrote:

Alan Cox <[email protected]> writes:

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

This function still depends on the big kernel lock in some cases. Push locking into the function ready for removal of the BKL from ioctl call paths.

Didn't you forget the .ioctl -> .unlocked_ioctl change?

We are not yet ready to unlock the device ioctl paths for tty. We still

Traditionally the usual is to first convert .ioctl to .unlocked_ioctl and just slap lock_kernel around the whole ioctl handler and then later move it down step by step.

I didn't read the code completely but I assume tty_ioctl would be that handler. I guess i was wrong?

-Andi

#4 lore raw
Quoted 9 lines · Andi Kleen
Quoted 8 lines · Alan Cox
Quoted 5 lines · On 10 Mar 2008 23:12:51 +0100 Andi Kleen <[email protected]> wrote:
Quoted 3 lines · Alan Cox

This function still depends on the big kernel lock in some cases. Push locking into the function ready for removal of the BKL from ioctl call paths.

Quoted 8 lines
Quoted 2 lines
Quoted 1 line

Didn't you forget the .ioctl -> .unlocked_ioctl change?

We are not yet ready to unlock the device ioctl paths for tty. We still

Traditionally the usual is to first convert .ioctl to .unlocked_ioctl
and just slap lock_kernel around the whole ioctl handler and then later move it down step by step.

I didn't read the code completely but I assume tty_ioctl would be that handler. I guess i was wrong?

tty_ioctl is already unlocked_ioctl (in -mm) ;). These patches are those next steps to propagate the bkl down.

regards,
--js