From owner-freebsd-arch@FreeBSD.ORG Thu Mar 13 13:50:38 2008 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2C0FD1065705 for ; Thu, 13 Mar 2008 13:50:38 +0000 (UTC) (envelope-from ed@hoeg.nl) Received: from palm.hoeg.nl (mx0.hoeg.nl [IPv6:2001:610:652::211]) by mx1.freebsd.org (Postfix) with ESMTP id DBB818FC1B for ; Thu, 13 Mar 2008 13:50:36 +0000 (UTC) (envelope-from ed@hoeg.nl) Received: by palm.hoeg.nl (Postfix, from userid 1000) id 567711CC44; Thu, 13 Mar 2008 14:50:35 +0100 (CET) Date: Thu, 13 Mar 2008 14:50:35 +0100 From: Ed Schouten To: FreeBSD Arch Message-ID: <20080313135035.GB80576@hoeg.nl> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ht9V8wKec6a3w1Ef" Content-Disposition: inline User-Agent: Mutt/1.5.17 (2007-11-01) Cc: Subject: New TTY layer: condvar(9) and Giant X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Mar 2008 13:50:38 -0000 --ht9V8wKec6a3w1Ef Content-Type: multipart/mixed; boundary="FexDM9E/OpjgUmaq" Content-Disposition: inline --FexDM9E/OpjgUmaq Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hello everyone, Almost a month ago I started working on my assignment for my internship, to reimplement a new TTY layer that fixes a lot of architectural problems. So far, things are going quite fast: - I've already implemented a basic TTY layer, which has support for canonical and non-canonical mode. It still misses important features including flow control, but it seems to work quite good. Unlike the old layer, it doesn't buffer data as much, which should hopefully mean it's a bit faster. - I'm using a new PTY driver called pts(4). It works quite good, but it misses the compatibility bits, which we'll need to have to support older FreeBSD or Linux binaries. - Some of you may have read I'm working on syscons now. I've got syscons working with the new TTY layer; I'm typing this message through syscons. ;-) A lot of drivers that are used by the old TTY layer aren't mpsafe yet. Of course, I'm willing to fix this, but this cannot be done in the nearby future. This is why the new TTY layer should still allow TTY's to be run under Giant. In my initial implementation, each TTY device had its own mutex. In theory, this is great. The PTY driver already uses this and it works fine. There will be a lot of drivers, however, that want to use a per-class mutex to lock all related TTY devices down at once (i.e. syscons, which allocates 16 virtual TTY's). This is why I introduced a per-class lock. When set to Giant, all TTY instances will lock down the Giant lock when entering the TTY layer. Unfortunately, I discovered condvar(9) can't properly unlock/lock the Giant, which causes the system to panic. The condvar routines already call DROP_GIANT before unlocking the lock itself. I've attached a patch that adds support for Giant to condvar(9). I had to patch sys/mutex.h a little, because we now only need to call DROP_GIANT() under certain conditions. The macro's didn't allow that, because DROP_GIANT starts a new code block. I'm sending this to arch@, because I want to know if I'm doing something silly. It seems to work properly on my machine, but I'm not an SMP expert. ;-) --=20 Ed Schouten WWW: http://g-rave.nl/ --FexDM9E/OpjgUmaq Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="condvar-giant.diff" Content-Transfer-Encoding: quoted-printable --- sys/kern/kern_condvar.c +++ sys/kern/kern_condvar.c @@ -95,6 +95,7 @@ _cv_wait(struct cv *cvp, struct lock_object *lock) { WITNESS_SAVE_DECL(lock_witness); + PARTIAL_DROP_GIANT_DECL(); struct lock_class *class; struct thread *td; int lock_state; @@ -123,7 +124,8 @@ sleepq_lock(cvp); =20 cvp->cv_waiters++; - DROP_GIANT(); + if (lock !=3D (struct lock_object *)&Giant) + PARTIAL_DROP_GIANT(); =20 sleepq_add(cvp, lock, cvp->cv_description, SLEEPQ_CONDVAR, 0); if (class->lc_flags & LC_SLEEPABLE) @@ -137,7 +139,8 @@ if (KTRPOINT(td, KTR_CSW)) ktrcsw(0, 0); #endif - PICKUP_GIANT(); + if (lock !=3D (struct lock_object *)&Giant) + PARTIAL_PICKUP_GIANT(); class->lc_lock(lock, lock_state); WITNESS_RESTORE(lock, lock_witness); } @@ -149,6 +152,7 @@ void _cv_wait_unlock(struct cv *cvp, struct lock_object *lock) { + PARTIAL_DROP_GIANT_DECL(); struct lock_class *class; struct thread *td; =20 @@ -176,7 +180,8 @@ sleepq_lock(cvp); =20 cvp->cv_waiters++; - DROP_GIANT(); + if (lock !=3D (struct lock_object *)&Giant) + PARTIAL_DROP_GIANT(); =20 sleepq_add(cvp, lock, cvp->cv_description, SLEEPQ_CONDVAR, 0); if (class->lc_flags & LC_SLEEPABLE) @@ -190,7 +195,8 @@ if (KTRPOINT(td, KTR_CSW)) ktrcsw(0, 0); #endif - PICKUP_GIANT(); + if (lock !=3D (struct lock_object *)&Giant) + PARTIAL_PICKUP_GIANT(); } =20 /* @@ -203,6 +209,7 @@ _cv_wait_sig(struct cv *cvp, struct lock_object *lock) { WITNESS_SAVE_DECL(lock_witness); + PARTIAL_DROP_GIANT_DECL(); struct lock_class *class; struct thread *td; struct proc *p; @@ -233,7 +240,8 @@ sleepq_lock(cvp); =20 cvp->cv_waiters++; - DROP_GIANT(); + if (lock !=3D (struct lock_object *)&Giant) + PARTIAL_DROP_GIANT(); =20 sleepq_add(cvp, lock, cvp->cv_description, SLEEPQ_CONDVAR | SLEEPQ_INTERRUPTIBLE, 0); @@ -248,7 +256,8 @@ if (KTRPOINT(td, KTR_CSW)) ktrcsw(0, 0); #endif - PICKUP_GIANT(); + if (lock !=3D (struct lock_object *)&Giant) + PARTIAL_PICKUP_GIANT(); class->lc_lock(lock, lock_state); WITNESS_RESTORE(lock, lock_witness); =20 @@ -264,6 +273,7 @@ _cv_timedwait(struct cv *cvp, struct lock_object *lock, int timo) { WITNESS_SAVE_DECL(lock_witness); + PARTIAL_DROP_GIANT_DECL(); struct lock_class *class; struct thread *td; int lock_state, rval; @@ -293,7 +303,8 @@ sleepq_lock(cvp); =20 cvp->cv_waiters++; - DROP_GIANT(); + if (lock !=3D (struct lock_object *)&Giant) + PARTIAL_DROP_GIANT(); =20 sleepq_add(cvp, lock, cvp->cv_description, SLEEPQ_CONDVAR, 0); sleepq_set_timeout(cvp, timo); @@ -308,7 +319,8 @@ if (KTRPOINT(td, KTR_CSW)) ktrcsw(0, 0); #endif - PICKUP_GIANT(); + if (lock !=3D (struct lock_object *)&Giant) + PARTIAL_PICKUP_GIANT(); class->lc_lock(lock, lock_state); WITNESS_RESTORE(lock, lock_witness); =20 @@ -325,6 +337,7 @@ _cv_timedwait_sig(struct cv *cvp, struct lock_object *lock, int timo) { WITNESS_SAVE_DECL(lock_witness); + PARTIAL_DROP_GIANT_DECL(); struct lock_class *class; struct thread *td; struct proc *p; @@ -356,7 +369,8 @@ sleepq_lock(cvp); =20 cvp->cv_waiters++; - DROP_GIANT(); + if (lock !=3D (struct lock_object *)&Giant) + PARTIAL_DROP_GIANT(); =20 sleepq_add(cvp, lock, cvp->cv_description, SLEEPQ_CONDVAR | SLEEPQ_INTERRUPTIBLE, 0); @@ -372,7 +386,8 @@ if (KTRPOINT(td, KTR_CSW)) ktrcsw(0, 0); #endif - PICKUP_GIANT(); + if (lock !=3D (struct lock_object *)&Giant) + PARTIAL_PICKUP_GIANT(); class->lc_lock(lock, lock_state); WITNESS_RESTORE(lock, lock_witness); =20 --- sys/sys/mutex.h +++ sys/sys/mutex.h @@ -368,26 +368,33 @@ #ifndef DROP_GIANT #define DROP_GIANT() \ do { \ + PARTIAL_DROP_GIANT_DECL(); \ + PARTIAL_DROP_GIANT(); + +#define PARTIAL_DROP_GIANT_DECL() \ int _giantcnt =3D 0; \ - WITNESS_SAVE_DECL(Giant); \ - \ + WITNESS_SAVE_DECL(Giant); + +#define PARTIAL_DROP_GIANT() do { \ if (mtx_owned(&Giant)) { \ WITNESS_SAVE(&Giant.lock_object, Giant); \ for (_giantcnt =3D 0; mtx_owned(&Giant); _giantcnt++) \ mtx_unlock(&Giant); \ - } + } \ +} while (0) =20 #define PICKUP_GIANT() \ PARTIAL_PICKUP_GIANT(); \ } while (0) =20 -#define PARTIAL_PICKUP_GIANT() \ +#define PARTIAL_PICKUP_GIANT() do { \ mtx_assert(&Giant, MA_NOTOWNED); \ if (_giantcnt > 0) { \ while (_giantcnt--) \ mtx_lock(&Giant); \ WITNESS_RESTORE(&Giant.lock_object, Giant); \ - } + } \ +} while(0) #endif =20 #define UGAR(rval) do { \ --FexDM9E/OpjgUmaq-- --ht9V8wKec6a3w1Ef Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.8 (FreeBSD) iEYEARECAAYFAkfZMSsACgkQ52SDGA2eCwXMFQCfYwaKBVHU7xCBZv/D+yglHfmk 7dEAn1mXTouuc66FGFTiiVnM6ylfLri5 =5MNx -----END PGP SIGNATURE----- --ht9V8wKec6a3w1Ef--