LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] tty_ioctl: locking for tty_wait_until_sent
@ 2008-03-10 21:53 Alan Cox
  2008-03-10 22:12 ` Andi Kleen
  0 siblings, 1 reply; 5+ messages in thread
From: Alan Cox @ 2008-03-10 21:53 UTC (permalink / raw)
  To: akpm, linux-kernel

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 <alan@redhat.com>

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
--- linux.vanilla-2.6.25-rc3-mm1/drivers/char/tty_ioctl.c	2008-03-10 12:57:53.000000000 +0000
+++ linux-2.6.25-rc3-mm1/drivers/char/tty_ioctl.c	2008-03-10 13:27:09.000000000 +0000
@@ -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);
 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tty_ioctl: locking for tty_wait_until_sent
  2008-03-10 22:12 ` Andi Kleen
@ 2008-03-10 22:06   ` Alan Cox
  2008-03-10 22:28     ` Andi Kleen
  0 siblings, 1 reply; 5+ messages in thread
From: Alan Cox @ 2008-03-10 22:06 UTC (permalink / raw)
  To: Andi Kleen; +Cc: akpm, linux-kernel

On 10 Mar 2008 23:12:51 +0100
Andi Kleen <andi@firstfloor.org> wrote:

> Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
> 
> > 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.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tty_ioctl: locking for tty_wait_until_sent
  2008-03-10 21:53 [PATCH] tty_ioctl: locking for tty_wait_until_sent Alan Cox
@ 2008-03-10 22:12 ` Andi Kleen
  2008-03-10 22:06   ` Alan Cox
  0 siblings, 1 reply; 5+ messages in thread
From: Andi Kleen @ 2008-03-10 22:12 UTC (permalink / raw)
  To: Alan Cox; +Cc: akpm, linux-kernel

Alan Cox <alan@lxorguk.ukuu.org.uk> writes:

> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tty_ioctl: locking for tty_wait_until_sent
  2008-03-10 22:06   ` Alan Cox
@ 2008-03-10 22:28     ` Andi Kleen
  2008-03-10 23:16       ` Jiri Slaby
  0 siblings, 1 reply; 5+ messages in thread
From: Andi Kleen @ 2008-03-10 22:28 UTC (permalink / raw)
  To: Alan Cox; +Cc: Andi Kleen, akpm, linux-kernel

On Mon, Mar 10, 2008 at 10:06:43PM +0000, Alan Cox wrote:
> On 10 Mar 2008 23:12:51 +0100
> Andi Kleen <andi@firstfloor.org> wrote:
> 
> > Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
> > 
> > > 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tty_ioctl: locking for tty_wait_until_sent
  2008-03-10 22:28     ` Andi Kleen
@ 2008-03-10 23:16       ` Jiri Slaby
  0 siblings, 0 replies; 5+ messages in thread
From: Jiri Slaby @ 2008-03-10 23:16 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Alan Cox, akpm, linux-kernel

On 03/10/2008 11:28 PM, Andi Kleen wrote:
> On Mon, Mar 10, 2008 at 10:06:43PM +0000, Alan Cox wrote:
>> On 10 Mar 2008 23:12:51 +0100
>> Andi Kleen <andi@firstfloor.org> wrote:
>>
>>> Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
>>>
>>>> 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?

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

regards,
--js

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-03-10 23:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-03-10 21:53 [PATCH] tty_ioctl: locking for tty_wait_until_sent Alan Cox
2008-03-10 22:12 ` Andi Kleen
2008-03-10 22:06   ` Alan Cox
2008-03-10 22:28     ` Andi Kleen
2008-03-10 23:16       ` Jiri Slaby

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).