Date: Sat, 8 Sep 2012 18:38:39 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Attilio Rao <attilio@freebsd.org> Cc: Davide Italiano <davide@freebsd.org>, svn-src-projects@freebsd.org, src-committers@freebsd.org, Andriy Gapon <avg@freebsd.org> Subject: Re: svn commit: r238907 - projects/calloutng/sys/kern Message-ID: <20120908153839.GF33100@deviant.kiev.zoral.com.ua> In-Reply-To: <CAJ-FndDAFmrEBW3d29cj-CZoPT%2BD5UPFadzL6i9BNH9ztzsJ%2BQ@mail.gmail.com> References: <CACYV=-HmOwZ=E8Pw3-mUw0994SbvZaA3eMfcwM0fDTu_zykBJg@mail.gmail.com> <CAJ-FndBmXkyJJ=fCkEpVm84E56A2_EoM6kbch03e4RMEM6WCGQ@mail.gmail.com> <20120730143943.GY2676@deviant.kiev.zoral.com.ua> <CAJ-FndByYcZ%2BUhnkFT_n2=W=UheqUCi0%2BUAX%2BF07EqbVU=6iDQ@mail.gmail.com> <CAJ-FndCQ6HGAfFdjofNfJ%2BHeNaE7uqoNhJB9GH4pGFxyZ_1yLg@mail.gmail.com> <5016A21B.6090409@FreeBSD.org> <CAJ-FndCFjZP=0ThpMxy6WSDQAZOm0TRkyu0bWfxVBwtT-h%2B1cA@mail.gmail.com> <5016A8E4.7070405@FreeBSD.org> <CAJ-FndB0zc-TC0E=H4p1qcOB4ngEWtwXoyhScf68G8i0p5UErw@mail.gmail.com> <CAJ-FndDAFmrEBW3d29cj-CZoPT%2BD5UPFadzL6i9BNH9ztzsJ%2BQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--Cxx16vhE7hPQXf14 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 07, 2012 at 11:35:15PM +0100, Attilio Rao wrote: > On Mon, Jul 30, 2012 at 9:39 PM, Attilio Rao <attilio@freebsd.org> wrote: > > On Mon, Jul 30, 2012 at 4:31 PM, Andriy Gapon <avg@freebsd.org> wrote: > >> on 30/07/2012 18:04 Attilio Rao said the following: > >>> On 7/30/12, Andriy Gapon <avg@freebsd.org> wrote: > >>>> on 30/07/2012 17:56 Attilio Rao said the following: > >>>>> More explicitly, I think such combination TDP_NOSLEEPING + > >>>>> TDP_NOBLOCKING (name invented) should be set on entering the interr= upt > >>>>> context, not only related to this part of callouts. This would be a > >>>>> very good help for catching buggy situations. > >>>> > >>>> Something very tangential. I think it would also be nice to check i= f a > >>>> thread has > >>>> any(?) locks held when returning to userland. > >>> > >>> This happens already for INVARIANTS case, with td_locks counters. > >>> In the !INVARIANTS case, this doesn't happen because you don't want to > >>> add the burden to bump td_locks for the fast case and I think it is a > >>> good approach. > >> > >> Ah, I missed that, thank you. > >> BTW, it seems that td_locks is checked twice in normal syscallret() pa= th: once in > >> syscallret() itself and then in userret(). On this note, would it mak= e sense to > >> move the whole nine yards of asserts from syscallret() to userret()? > >> I mean it might make sense to have those checks (td_critnest, td_pflag= s) in other > >> paths to userland. > > > > Nice catch. > > The checks were added to syscallret() in r208453. While this is fine, > > I think that putting them in userret() may give them more exposure and > > cover also cases like traps which are not covered right now. > > If you want to make a patch that moves these conditions in userret() > > I'd be in favor of it. >=20 > More specifically, what do you think about this patch?: > http://www.freebsd.org/~attilio/userret_diag.patch >=20 > Of course I moved the XEN par too before the checks. > The patch survived to few consecutive and parallel buildworlds, FWIW. At least in fork_return(), the last assert which checks that Giant is not held, is no longer needed. I had similar thought from the time when I added TDP_NOFAULTING check to the syscallret(), but the loss of the syscall name in the panic output always stopped me. --Cxx16vhE7hPQXf14 Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (FreeBSD) iEYEARECAAYFAlBLZn8ACgkQC3+MBN1Mb4ikzACg8s+t4a+wwanhpiFRFacRjuIZ IzcAoPZ+mhAnSotFwYjMeVj574WW9ier =xZiz -----END PGP SIGNATURE----- --Cxx16vhE7hPQXf14--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120908153839.GF33100>