From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Hurley Subject: [PATCH v4 08/32] tty: Strengthen no-subsequent-use guarantee of tty_ldisc_halt() Date: Wed, 20 Feb 2013 15:02:55 -0500 Message-ID: <1361390599-15195-9-git-send-email-peter@hurleysoftware.com> References: <1361390599-15195-1-git-send-email-peter@hurleysoftware.com> Return-path: Received: from mailout02.c08.mtsvc.net ([205.186.168.190]:54012 "EHLO mailout02.c08.mtsvc.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935358Ab3BTUFP (ORCPT ); Wed, 20 Feb 2013 15:05:15 -0500 In-Reply-To: <1361390599-15195-1-git-send-email-peter@hurleysoftware.com> In-Reply-To: <1360095638-6624-1-git-send-email-peter@hurleysoftware.com> References: <1360095638-6624-1-git-send-email-peter@hurleysoftware.com> Sender: linux-serial-owner@vger.kernel.org List-Id: linux-serial@vger.kernel.org To: Greg Kroah-Hartman , Jiri Slaby Cc: Sasha Levin , Sebastian Andrzej Siewior , linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org, Ilya Zykov , Dave Jones , Michael Ellerman , Shawn Guo , Peter Hurley In preparation for destructing and freeing the tty, the line discipline must first be brought to an inactive state before it can be destructed. This line discipline shutdown must: - disallow new users of the ldisc - wait for existing ldisc users to finish - only then, cancel/flush their pending/running work Factor tty_ldisc_wait_idle() from tty_set_ldisc() and tty_ldisc_kill() to ensure this shutdown order. Failure to provide this guarantee can result in scheduled work running after the tty has already been freed, as indicated in the following log message: [ 88.331234] WARNING: at drivers/tty/tty_buffer.c:435 flush_to_ldisc+0x194/0x1d0() [ 88.334505] Hardware name: Bochs [ 88.335618] tty is bad=-1 [ 88.335703] Modules linked in: netconsole configfs bnep rfcomm bluetooth ...... [ 88.345272] Pid: 39, comm: kworker/1:1 Tainted: G W 3.7.0-next-20121129+ttydebug-xeon #20121129+ttydebug [ 88.347736] Call Trace: [ 88.349024] [] warn_slowpath_common+0x7f/0xc0 [ 88.350383] [] warn_slowpath_fmt+0x46/0x50 [ 88.351745] [] flush_to_ldisc+0x194/0x1d0 [ 88.353047] [] ? _raw_spin_unlock_irq+0x21/0x50 [ 88.354190] [] ? finish_task_switch+0x49/0xe0 [ 88.355436] [] process_one_work+0x121/0x490 [ 88.357674] [] ? __tty_buffer_flush+0x90/0x90 [ 88.358954] [] worker_thread+0x164/0x3e0 [ 88.360247] [] ? manage_workers+0x120/0x120 [ 88.361282] [] kthread+0xc0/0xd0 [ 88.362284] [] ? cmos_do_probe+0x2eb/0x3bf [ 88.363391] [] ? flush_kthread_worker+0xb0/0xb0 [ 88.364797] [] ret_from_fork+0x7c/0xb0 [ 88.366087] [] ? flush_kthread_worker+0xb0/0xb0 [ 88.367266] ---[ end trace 453a7c9f38fbfec0 ]--- Signed-off-by: Peter Hurley --- drivers/tty/tty_ldisc.c | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c index f691c76..525ee53 100644 --- a/drivers/tty/tty_ldisc.c +++ b/drivers/tty/tty_ldisc.c @@ -530,24 +530,38 @@ static int tty_ldisc_wait_idle(struct tty_struct *tty, long timeout) /** * tty_ldisc_halt - shut down the line discipline * @tty: tty device + * @pending: returns true if work was scheduled when cancelled + * (can be set to NULL) + * @timeout: # of jiffies to wait for ldisc refs to be released * * Shut down the line discipline and work queue for this tty device. * The TTY_LDISC flag being cleared ensures no further references can * be obtained while the delayed work queue halt ensures that no more * data is fed to the ldisc. * + * Furthermore, guarantee that existing ldisc references have been + * released, which in turn, guarantees that no future buffer work + * can be rescheduled. + * * You need to do a 'flush_scheduled_work()' (outside the ldisc_mutex) * in order to make sure any currently executing ldisc work is also * flushed. */ -static int tty_ldisc_halt(struct tty_struct *tty) +static int tty_ldisc_halt(struct tty_struct *tty, int *pending, long timeout) { - int scheduled; + int scheduled, retval; + clear_bit(TTY_LDISC, &tty->flags); + retval = tty_ldisc_wait_idle(tty, timeout); + if (retval) + return retval; + scheduled = cancel_work_sync(&tty->port->buf.work); set_bit(TTY_LDISC_HALTED, &tty->flags); - return scheduled; + if (pending) + *pending = scheduled; + return 0; } /** @@ -688,9 +702,9 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc) * parallel to the change and re-referencing the tty. */ - work = tty_ldisc_halt(tty); - if (o_tty) - o_work = tty_ldisc_halt(o_tty); + retval = tty_ldisc_halt(tty, &work, 5 * HZ); + if (!retval && o_tty) + retval = tty_ldisc_halt(o_tty, &o_work, 5 * HZ); /* * Wait for ->hangup_work and ->buf.work handlers to terminate. @@ -701,8 +715,6 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc) tty_ldisc_flush_works(tty); - retval = tty_ldisc_wait_idle(tty, 5 * HZ); - tty_lock(tty); mutex_lock(&tty->ldisc_mutex); @@ -921,11 +933,6 @@ int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty) static void tty_ldisc_kill(struct tty_struct *tty) { - /* There cannot be users from userspace now. But there still might be - * drivers holding a reference via tty_ldisc_ref. Do not steal them the - * ldisc until they are done. */ - tty_ldisc_wait_idle(tty, MAX_SCHEDULE_TIMEOUT); - mutex_lock(&tty->ldisc_mutex); /* * Now kill off the ldisc @@ -958,13 +965,12 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty) * race with the set_ldisc code path. */ - tty_ldisc_halt(tty); - if (o_tty) - tty_ldisc_halt(o_tty); - + tty_ldisc_halt(tty, NULL, MAX_SCHEDULE_TIMEOUT); tty_ldisc_flush_works(tty); - if (o_tty) + if (o_tty) { + tty_ldisc_halt(o_tty, NULL, MAX_SCHEDULE_TIMEOUT); tty_ldisc_flush_works(o_tty); + } tty_lock_pair(tty, o_tty); /* This will need doing differently if we need to lock */ -- 1.8.1.2