1. 10 Aug, 2012 1 commit
    • Alan Cox's avatar
      tty: localise the lock · 89c8d91e
      Alan Cox authored
      The termios and other changes mean the other protections needed on the driver
      tty arrays should be adequate. Turn it all back on.
      
      This contains pieces folded in from the fixes made to the original patches
      
      | From: Geert Uytterhoeven <geert@linux-m68k.org>	(fix m68k)
      | From: Paul Gortmaker <paul.gortmaker@windriver.com>	(fix cris)
      | From: Jiri Kosina <jkosina@suze.cz>			(lockdep)
      | From: Eric Dumazet <eric.dumazet@gmail.com>		(lockdep)
      Signed-off-by: default avatarAlan Cox <alan@linux.intel.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      89c8d91e
  2. 27 Jul, 2012 1 commit
    • Alan Cox's avatar
      tty: Fix race in tty release · d155255a
      Alan Cox authored
      Ian Abbott found that the tty layer would explode with the right set of
      parallel open and close operations. This is because we race in the
      handling of tty->drivers->termios[].
      
      Correct this by
      	Making tty_ldisc_release behave like nromal code (takes the lock,
      			does stuff, drops the lock)
      	Drop the tty lock earlier in tty_ldisc_release
      	Taking the tty mutex around the driver->termios update in all cases
      	Adding a WARN_ON to catch future screwups.
      
      I also forgot to clean up the pty resources properly. With a pty pair we
      need to pull both halves out of the tables.
      Signed-off-by: default avatarAlan Cox <alan@linux.intel.com>
      Tested-by: default avatarIan Abbott <abbotti@mev.co.uk>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      d155255a
  3. 26 Jul, 2012 1 commit
  4. 16 Jul, 2012 2 commits
  5. 12 Jul, 2012 1 commit
  6. 06 Jul, 2012 1 commit
    • Alan Cox's avatar
      tty: localise the lock · f5e3bcc5
      Alan Cox authored
      The termios and other changes mean the other protections needed on the driver
      tty arrays should be adequate. Turn it all back on.
      
      This contains pieces folded in from the fixes made to the original patches
      
      | From: Geert Uytterhoeven <geert@linux-m68k.org>	(fix m68k)
      | From: Paul Gortmaker <paul.gortmaker@windriver.com>	(fix cris)
      | From: Jiri Kosina <jkosina@suze.cz>			(lockdep)
      | From: Eric Dumazet <eric.dumazet@gmail.com>		(lockdep)
      Signed-off-by: default avatarAlan Cox <alan@linux.intel.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      f5e3bcc5
  7. 02 Jun, 2012 1 commit
    • Linus Torvalds's avatar
      tty: Revert the tty locking series, it needs more work · f309532b
      Linus Torvalds authored
      This reverts the tty layer change to use per-tty locking, because it's
      not correct yet, and fixing it will require some more deep surgery.
      
      The main revert is d29f3ef3 ("tty_lock: Localise the lock"), but
      there are several smaller commits that built upon it, they also get
      reverted here. The list of reverted commits is:
      
        fde86d31 - tty: add lockdep annotations
        8f6576ad - tty: fix ldisc lock inversion trace
        d3ca8b64 - pty: Fix lock inversion
        b1d679af - tty: drop the pty lock during hangup
        abcefe5f - tty/amiserial: Add missing argument for tty_unlock()
        fd11b42e - cris: fix missing tty arg in wait_event_interruptible_tty call
        d29f3ef3 - tty_lock: Localise the lock
      
      The revert had a trivial conflict in the 68360serial.c staging driver
      that got removed in the meantime.
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      f309532b
  8. 29 May, 2012 1 commit
  9. 10 May, 2012 1 commit
  10. 04 May, 2012 1 commit
  11. 17 Nov, 2011 4 commits
    • Jiri Slaby's avatar
      TTY: ldisc, remove some unneeded includes · 8b3ffa17
      Jiri Slaby authored
      They were cut&pasted from tty_io. Many of them are not needed in
      tty_ldisc.
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      Cc: Alan Cox <alan@linux.intel.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      8b3ffa17
    • Jiri Slaby's avatar
      TTY: ldisc, wait for ldisc infinitely in hangup · 0c73c08e
      Jiri Slaby authored
      For /dev/console case, we do not kill all ldisc users. It's due to
      redirected_tty_write test in __tty_hangup. In that case there still
      might be a process waiting e.g. in n_tty_read for input.
      
      We wait for such processes to disappear. The problem is that we use a
      timeout. After this timeout, we continue closing the ldisc and start
      freeing tty resources. It obviously leads to crashes when the other
      process is woken.
      
      So to fix this, we wait infinitely before reiniting the ldisc. (The
      tiocsetd remains untouched -- times out after 5s.)
      
      This is nicely reproducible with this run from shell:
        exec 0<>/dev/console 1<>/dev/console 2<>/dev/console
      and stopping a getty like:
        systemctl stop serial-getty@ttyS0.service
      
      The crash proper may be produced only under load or with constified
      timing the same as for 92f6fa09.
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      Cc: Dave Young <hidave.darkstar@gmail.com>
      Cc: Dave Jones <davej@redhat.com>
      Cc: Ben Hutchings <ben@decadent.org.uk>
      Cc: Dmitriy Matrosov <sgf.dma@gmail.com>
      Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
      Cc: stable <stable@vger.kernel.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      0c73c08e
    • Jiri Slaby's avatar
      TTY: ldisc, move wait idle to caller · 30042072
      Jiri Slaby authored
      It is the only place where reinit is called from. And we really need
      to wait for the old ldisc to go once. Actually this is the place where
      the waiting originally was (before removed and re-added later).
      
      This will make the fix in the following patch easier to implement.
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      Cc: Dave Young <hidave.darkstar@gmail.com>
      Cc: Dave Jones <davej@redhat.com>
      Cc: Ben Hutchings <ben@decadent.org.uk>
      Cc: Dmitriy Matrosov <sgf.dma@gmail.com>
      Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
      Cc: stable <stable@vger.kernel.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      30042072
    • Jiri Slaby's avatar
      TTY: ldisc, allow waiting for ldisc arbitrarily long · df92d056
      Jiri Slaby authored
      To fix a nasty bug in ldisc hup vs. reinit we need to wait infinitely
      long for ldisc to be gone. So here we add a parameter to
      tty_ldisc_wait_idle to allow that.
      
      This is only a preparation for the real fix which is done in the
      following patches.
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      Cc: Dave Young <hidave.darkstar@gmail.com>
      Cc: Dave Jones <davej@redhat.com>
      Cc: Ben Hutchings <ben@decadent.org.uk>
      Cc: Dmitriy Matrosov <sgf.dma@gmail.com>
      Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
      Cc: stable <stable@vger.kernel.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      df92d056
  12. 15 Nov, 2011 1 commit
  13. 23 Aug, 2011 1 commit
  14. 07 Jun, 2011 1 commit
    • Jiri Slaby's avatar
      TTY: ldisc, do not close until there are readers · 92f6fa09
      Jiri Slaby authored
      We restored tty_ldisc_wait_idle in 100eeae2 (TTY: restore
      tty_ldisc_wait_idle). We used it in the ldisc changing path to fix the
      case where there are tasks in n_tty_read waiting for data and somebody
      tries to change ldisc.
      
      Similar to the case above, there may be also tasks waiting in
      n_tty_read while hangup is performed. As 65b77046 (tty-ldisc: turn
      ldisc user count into a proper refcount) removed the wait-until-idle
      from all paths, hangup path won't wait for them to disappear either
      now. So add it back even to the hangup path.
      
      There is a difference, we need uninterruptible sleep as there is
      obviously HUP signal pending. So tty_ldisc_wait_idle now sleeps
      without possibility to be interrupted. This is what original
      tty_ldisc_wait_idle did. After the wait idle reintroduction
      (100eeae2), we have had interruptible sleeps for the ldisc changing
      path. But as there is a 5s timeout anyway, we don't allow it to be
      interrupted from now on. It's not worth the added complexity of
      deciding what kind of sleep we want.
      
      Before 65b77046 tty_ldisc_release was called also from
      tty_ldisc_release. It is called from tty_release, so I don't think we
      need to restore that one.
      
      This is nicely reproducible after constifying the timing when
      drivers/tty/n_tty.c is patched as follows ("TTY: ntty, add one more
      sanity check" patch is needed to actually see it explode):
      %% -1548,6 +1549,7 @@ static int n_tty_open(struct tty_struct *tty)
      
              /* These are ugly. Currently a malloc failure here can panic */
              if (!tty->read_buf) {
      +               msleep(100);
                      tty->read_buf = kzalloc(N_TTY_BUF_SIZE, GFP_KERNEL);
                      if (!tty->read_buf)
                              return -ENOMEM;
      %% -1785,6 +1788,7 @@ do_it_again:
                                      break;
                              }
                              timeout = schedule_timeout(timeout);
      +                       msleep(20);
                              continue;
                      }
                      __set_current_state(TASK_RUNNING);
      ===== With a process: =====
          while (1) {
              int fd = open(argv[1], O_RDWR);
              read(fd, buf, sizeof(buf));
              close(fd);
          }
      ===== and its child: =====
              setsid();
              while (1) {
                      int fd = open(tty, O_RDWR|O_NOCTTY);
                      ioctl(fd, TIOCSCTTY, 1);
                      vhangup();
                      close(fd);
                      usleep(100 * (10 + random() % 1000));
              }
      ===== EOF =====
      
      References: https://bugzilla.novell.com/show_bug.cgi?id=693374
      References: https://bugzilla.novell.com/show_bug.cgi?id=694509Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
      Cc: Linus Torvalds <torvalds@linux-foundation.org>
      Cc: stable <stable@kernel.org> [32, 33, 34, 39]
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      92f6fa09
  15. 19 Apr, 2011 1 commit
    • Jiri Slaby's avatar
      TTY: introduce deinit helpers for proper ldisc shutdown · 6716671d
      Jiri Slaby authored
      Introduce deinitialize_tty_struct which should be called after
      initialize_tty_struct and before successfull tty_ldisc_setup.
      
      It calls tty_ldisc_deinit which is opposite of tty_ldisc_init. It only
      puts a reference to ldisc and assigns NULL to tty->ldisc.
      
      It will be used to shut down ldisc when tty_release cannot be called
      yet.
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
      Cc: Julian Anastasov <ja@ssi.bg>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      6716671d
  16. 22 Mar, 2011 1 commit
    • Linus Torvalds's avatar
      tty: stop using "delayed_work" in the tty layer · f23eb2b2
      Linus Torvalds authored
      Using delayed-work for tty flip buffers ends up causing us to wait for
      the next tick to complete some actions.  That's usually not all that
      noticeable, but for certain latency-critical workloads it ends up being
      totally unacceptable.
      
      As an extreme case of this, passing a token back-and-forth over a pty
      will take two ticks per iteration, so even just a thousand iterations
      will take 8 seconds assuming a common 250Hz configuration.
      
      Avoiding the whole delayed work issue brings that ping-pong test-case
      down to 0.009s on my machine.
      
      In more practical terms, this latency has been a performance problem for
      things like dive computer simulators (simulating the serial interface
      using the ptys) and for other environments (Alan mentions a CP/M emulator).
      Reported-by: default avatarJef Driesen <jefdriesen@telenet.be>
      Acked-by: default avatarGreg KH <gregkh@suse.de>
      Acked-by: default avatarAlan Cox <alan@lxorguk.ukuu.org.uk>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      f23eb2b2
  17. 01 Mar, 2011 1 commit
  18. 03 Feb, 2011 1 commit
  19. 29 Nov, 2010 1 commit
    • Jiri Slaby's avatar
      TTY: ldisc, fix open flag handling · 7f90cfc5
      Jiri Slaby authored
      When a concrete ldisc open fails in tty_ldisc_open, we forget to clear
      TTY_LDISC_OPEN. This causes a false warning on the next ldisc open:
      WARNING: at drivers/char/tty_ldisc.c:445 tty_ldisc_open+0x26/0x38()
      Hardware name: System Product Name
      Modules linked in: ...
      Pid: 5251, comm: a.out Tainted: G        W  2.6.32-5-686 #1
      Call Trace:
       [<c1030321>] ? warn_slowpath_common+0x5e/0x8a
       [<c1030357>] ? warn_slowpath_null+0xa/0xc
       [<c119311c>] ? tty_ldisc_open+0x26/0x38
       [<c11936c5>] ? tty_set_ldisc+0x218/0x304
      ...
      
      So clear the bit when failing...
      
      Introduced in c65c9bc3 (tty: rewrite the ldisc locking) back in
      2.6.31-rc1.
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      Cc: Alan Cox <alan@linux.intel.com>
      Reported-by: default avatarSergey Lapin <slapin@ossfans.org>
      Tested-by: default avatarSergey Lapin <slapin@ossfans.org>
      Cc: stable <stable@kernel.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      7f90cfc5
  20. 09 Nov, 2010 2 commits
    • Philippe Rétornaz's avatar
      tty_ldisc: Fix BUG() on hangup · 1c95ba1e
      Philippe Rétornaz authored
      A kernel BUG when bluetooth rfcomm connection drop while the associated
      serial port is open is sometime triggered.
      
      It seems that the line discipline can disappear between the
      tty_ldisc_put and tty_ldisc_get. This patch fall back to the N_TTY line
      discipline if the previous discipline is not available anymore.
      Signed-off-by: default avatarPhilippe Retornaz <philippe.retornaz@epfl.ch>
      Acked-by: default avatarAlan Cox <alan@linux.intel.com>
      Cc: stable <stable@kernel.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      1c95ba1e
    • Jiri Slaby's avatar
      TTY: restore tty_ldisc_wait_idle · 100eeae2
      Jiri Slaby authored
      It was removed in 65b77046 (tty-ldisc: turn ldisc user count into
      a proper refcount), but we need to wait for last user to quit the
      ldisc before we close it in tty_set_ldisc.
      
      Otherwise weird things start to happen. There might be processes
      waiting in tty_read->n_tty_read on tty->read_wait for input to appear
      and at that moment, a change of ldisc is fatal. n_tty_close is called,
      it frees read_buf and the waiting process is still in the middle of
      reading and goes nuts after it is woken.
      
      Previously we prevented close to happen when others are in ldisc ops
      by tty_ldisc_wait_idle in tty_set_ldisc. But the commit above removed
      that. So revoke the change and test whether there is 1 user (=we), and
      allow the close then.
      
      We can do that without ldisc/tty locks, because nobody else can open
      the device due to TTY_LDISC_CHANGING bit set, so we in fact wait for
      everybody to leave.
      
      I don't understand why tty_ldisc_lock would be needed either when the
      counter is an atomic variable, so this is a lockless
      tty_ldisc_wait_idle.
      
      On the other hand, if we fail to wait (timeout or signal), we have to
      reenable the halted ldiscs, so we take ldisc lock and reuse the setup
      path at the end of tty_set_ldisc.
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      Acked-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      Tested-by: default avatarSebastian Andrzej Siewior <bigeasy@breakpoint.cc>
      LKML-Reference: <20101031104136.GA511@Chamillionaire.breakpoint.cc>
      LKML-Reference: <1287669539-22644-1-git-send-email-jslaby@suse.cz>
      Cc: Alan Cox <alan@linux.intel.com>
      Cc: stable@kernel.org [32, 33, 36]
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      100eeae2
  21. 05 Nov, 2010 1 commit
  22. 10 Aug, 2010 3 commits
  23. 02 Mar, 2010 1 commit
    • Alan Cox's avatar
      tty: Fix the ldisc hangup race · 638b9648
      Alan Cox authored
      This was noticed by Matthias Urlichs and he proposed a fix. This patch
      does the fixing a different way to avoid introducing several new race
      conditions into the code.
      
      The problem case is TTY_DRIVER_RESET_TERMIOS = 0. In that case while we
      abort the ldisc change, the hangup processing has not cleaned up and restarted
      the ldisc either.
      
      We can't restart the ldisc stuff in the set_ldisc as we don't know what
      the hangup did and may touch stuff we shouldn't as we are no longer
      supposed to influence the tty at that point in case it has been re-opened
      before we get rescheduled.
      
      Instead do it the simple way. Always re-init the ldisc on the hangup, but
      use TTY_DRIVER_RESET_TERMIOS to indicate that we should force N_TTY.
      Signed-off-by: default avatarAlan Cox <alan@linux.intel.com>
      Cc: stable <stable@kernel.org>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      638b9648
  24. 11 Dec, 2009 2 commits
  25. 03 Oct, 2009 1 commit
    • Linus Torvalds's avatar
      tty: Avoid dropping ldisc_mutex over hangup tty re-initialization · 0b5759c6
      Linus Torvalds authored
      A couple of people have hit the WARN_ON() in drivers/char/tty_io.c,
      tty_open() that is unhappy about seeing the tty line discipline go away
      during the tty hangup. See for example
      
      	http://bugzilla.kernel.org/show_bug.cgi?id=14255
      
      and the reason is that we do the tty_ldisc_halt() outside the
      ldisc_mutex in order to be able to flush the scheduled work without a
      deadlock with vhangup_work.
      
      However, it turns out that we can solve this particular case by
      
       - using "cancel_delayed_work_sync()" in tty_ldisc_halt(), which waits
         for just the particular work, rather than synchronizing with any
         random outstanding pending work.
      
         This won't deadlock, since the buf.work we synchronize with doesn't
         care about the ldisc_mutex, it just flushes the tty ldisc buffers.
      
       - realize that for this particular case, we don't need to wait for any
         hangup work, because we are inside the hangup codepaths ourselves.
      
      so as a result we can just drop the flush_scheduled_work() entirely, and
      then move the tty_ldisc_halt() call to inside the mutex.  That way we
      never expose the partially torn down ldisc state to tty_open(), and hold
      the ldisc_mutex over the whole sequence.
      Reported-by: default avatarIngo Molnar <mingo@elte.hu>
      Reported-by: default avatarHeinz Diehl <htd@fancy-poultry.org>
      Cc: stable@kernel.org
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      0b5759c6
  26. 19 Sep, 2009 2 commits
  27. 25 Aug, 2009 1 commit
    • Linus Torvalds's avatar
      tty: make sure to flush any pending work when halting the ldisc · 5c58ceff
      Linus Torvalds authored
      When I rewrote tty ldisc code to use proper reference counts (commits
      65b77046 and cbe9352f) in order to avoid a race with hangup, the
      test-program that Eric Biederman used to trigger the original problem
      seems to have exposed another long-standing bug: the hangup code did the
      'tty_ldisc_halt()' to stop any buffer flushing activity, but unlike the
      other call sites it never actually flushed any pending work.
      
      As a result, if you get just the right timing, the pending work may be
      just about to execute (ie the timer has already triggered and thus
      cancel_delayed_work() was a no-op), when we then re-initialize the ldisc
      from under it.
      
      That, in turn, results in various random problems, usually seen as a
      NULL pointer dereference in run_timer_softirq() or a BUG() in
      worker_thread (but it can be almost anything).
      
      Fix it by adding the required 'flush_scheduled_work()' after doing the
      tty_ldisc_halt() (this also requires us to move the ldisc halt to before
      taking the ldisc mutex in order to avoid a deadlock with the workqueue
      executing do_tty_hangup, which requires the mutex).
      
      The locking should be cleaned up one day (the requirement to do this
      outside the ldisc_mutex is very annoying, and weakens the lock), but
      that's a larger and separate undertaking.
      Reported-by: default avatarEric W. Biederman <ebiederm@xmission.com>
      Tested-by: default avatarXiaotian Feng <xtfeng@gmail.com>
      Tested-by: default avatarYanmin Zhang <yanmin_zhang@linux.intel.com>
      Tested-by: default avatarDave Young <hidave.darkstar@gmail.com>
      Cc: Frederic Weisbecker <fweisbec@gmail.com>
      Cc: Greg Kroah-Hartman <gregkh@suse.de>
      Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      5c58ceff
  28. 04 Aug, 2009 3 commits
    • Linus Torvalds's avatar
      tty-ldisc: be more careful in 'put_ldisc' locking · cbe9352f
      Linus Torvalds authored
      Use 'atomic_dec_and_lock()' to make sure that we always hold the
      tty_ldisc_lock when the ldisc count goes to zero. That way we can never
      race against 'tty_ldisc_try()' increasing the count again.
      Reported-by: default avatarOGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      Tested-by: default avatarSergey Senozhatsky <sergey.senozhatsky@mail.by>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      cbe9352f
    • Linus Torvalds's avatar
      tty-ldisc: turn ldisc user count into a proper refcount · 65b77046
      Linus Torvalds authored
      By using the user count for the actual lifetime rules, we can get rid of
      the silly "wait_for_idle" logic, because any busy ldisc will
      automatically stay around until the last user releases it.  This avoids
      a host of odd issues, and simplifies the code.
      
      So now, when the last ldisc reference is dropped, we just release the
      ldisc operations struct reference, and free the ldisc.
      
      It looks obvious enough, and it does work for me, but the counting
      _could_ be off. It probably isn't (bad counting in the new version would
      generally imply that the old code did something really bad, like free an
      ldisc with a non-zero count), but it does need some testing, and
      preferably somebody looking at it.
      
      With this change, both 'tty_ldisc_put()' and 'tty_ldisc_deref()' are
      just aliases for the new ref-counting 'put_ldisc()'. Both of them
      decrement the ldisc user count and free it if it goes down to zero.
      They're identical functions, in other words.
      
      But the reason they still exist as sepate functions is that one of them
      was exported (tty_ldisc_deref) and had a stupid name (so I don't want to
      use it as the main name), and the other one was used in multiple places
      (and I didn't want to make the patch larger just to rename the users).
      
      In addition to the refcounting, I did do some minimal cleanup. For
      example, now "tty_ldisc_try()" actually returns the ldisc it got under
      the lock, rather than returning true/false and then the caller would
      look up the ldisc again (now without the protection of the lock).
      
      That said, there's tons of dubious use of 'tty->ldisc' without obviously
      proper locking or refcounting left. I expressly did _not_ want to try to
      fix it all, keeping the patch minimal. There may or may not be bugs in
      that kind of code, but they wouldn't be _new_ bugs.
      
      That said, even if the bugs aren't new, the timing and lifetime will
      change. For example, some silly code may depend on the 'tty->ldisc'
      pointer not changing because they hold a refcount on the 'ldisc'. And
      that's no longer true - if you hold a ref on the ldisc, the 'ldisc'
      itself is safe, but tty->ldisc may change.
      
      So the proper locking (remains) to hold tty->ldisc_mutex if you expect
      tty->ldisc to be stable. That's not really a _new_ rule, but it's an
      example of something that the old code might have unintentionally
      depended on and hidden bugs.
      
      Whatever. The patch _looks_ sensible to me. The only users of
      ldisc->users are:
       - get_ldisc() - atomically increment the count
      
       - put_ldisc() - atomically decrements the count and releases if zero
      
       - tty_ldisc_try_get() - creates the ldisc, and sets the count to 1.
         The ldisc should then either be released, or be attached to a tty.
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      Tested-by: default avatarOGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
      Tested-by: default avatarSergey Senozhatsky <sergey.senozhatsky@mail.by>
      Acked-by: default avatarAlan Cox <alan@linux.intel.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      65b77046
    • Linus Torvalds's avatar
      tty-ldisc: make refcount be atomic_t 'users' count · 18eac1cc
      Linus Torvalds authored
      This is pure preparation of changing the ldisc reference counting to be
      a true refcount that defines the lifetime of the ldisc.  But this is a
      purely syntactic change for now to make the next steps easier.
      
      This patch should make no semantic changes at all. But I wanted to make
      the ldisc refcount be an atomic (I will be touching it without locks
      soon enough), and I wanted to rename it so that there isn't quite as
      much confusion between 'ldo->refcount' (ldisk operations refcount) and
      'ld->refcount' (ldisc refcount itself) in the same file.
      
      So it's now an atomic 'ld->users' count. It still starts at zero,
      despite having a reference from 'tty->ldisc', but that will change once
      we turn it into a _real_ refcount.
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      Tested-by: default avatarOGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
      Tested-by: default avatarSergey Senozhatsky <sergey.senozhatsky@mail.by>
      Acked-by: default avatarAlan Cox <alan@linux.intel.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      18eac1cc
  29. 16 Jul, 2009 1 commit