Date: Thu, 16 Feb 2012 17:08:46 -0700 From: "Kenneth D. Merry" <ken@FreeBSD.org> To: Bruce Evans <brde@optusnet.com.au> Cc: marcel@FreeBSD.org, Eitan Adler <eadler@FreeBSD.org>, svn-src-all@FreeBSD.org, Andriy Gapon <avg@FreeBSD.org>, src-committers@FreeBSD.org, svn-src-head@FreeBSD.org, Julian Elischer <julian@FreeBSD.org> Subject: Re: svn commit: r231814 - in head/sys: kern sys Message-ID: <20120217000846.GA7641@nargothrond.kdm.org> In-Reply-To: <20120217053341.R1256@besplex.bde.org> References: <201202160511.q1G5BZNk099785@svn.freebsd.org> <20120216181210.K1423@besplex.bde.org> <4F3CC40D.4000307@freebsd.org> <4F3CC5C4.7020501@FreeBSD.org> <4F3CC8A5.3030107@FreeBSD.org> <20120216174758.GA64180@nargothrond.kdm.org> <20120217053341.R1256@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Feb 17, 2012 at 06:25:42 +1100, Bruce Evans wrote: > On Thu, 16 Feb 2012, Kenneth D. Merry wrote: > > >On Thu, Feb 16, 2012 at 11:13:09 +0200, Andriy Gapon wrote: > >>on 16/02/2012 11:00 Andriy Gapon said the following: > >>>on 16/02/2012 10:53 Julian Elischer said the following: > >>>>Bruce, this is a good example of a legitimate gripe going un-noticed > >>>>because > >>>>you didn't shout loud enough at the right time, at the right people. > >>>>It's been about 20 years since we started working on this but I've > >>>>finally > >>>>come to the point of saying that we need you to do more when you see > >>>>problems. > >>>>object officially if you think things should be backed out! > >>>> > >>>>your reasons here seem sound, so it's hard to see why you haven't been > >>>>more > >>>>public about it. > >>> > >>>Just for the record: Bruce and I voiced opinions against the commit when > >>>it went > >>>in after a rather short notice for such an important thing. The opinions > >>>can be found in the archives of these lists. > >> > >>To be more precise: s/opinions against/concerns about behavior in the > >>edge cases/. > >> > >>For me personally the immediate benefits in the common situations > >>outweighed the > >>problems in the edge cases, although I still believe that we can get the > >>former > >>without sacrifices in the latter. > > > >I will happily back the message buffer locking changes out if you and Bruce > >agree that that is the best thing to do. > > Thanks. I'm not sure what backing it out would take us back to. In theory, what I'm suggesting is backing out r222537, r222550, and probably r231814 (which is the one that started this most recent thread), since I think it depends on the previous two changes. It looks like r226435 (done by marcel) would also need to be removed since I think it was to fix bugs introduced in my changes. So I think we need some comment from Eitan on r231814, since that would probably need to come out as well. If he is okay with backing out r231814, and avg agrees as well, then I will back out all four changes. > >I can also review any proposed fixes to the message buffer locking code, > >but I really don't have the bandwidth right now to come up with the "real" > >solution. > > > >Sorry I didn't get around to dealing with it, I should have said something > >months ago. > > > >Now that you know a fix won't be coming from me, you two can let me know > >whether you'd like the changes backed out, or you are certainly free to > >commit a solution yourselves. > > I seem to remember that I asked you to test my old simple changes for > printf serialization on your larger systems that apparently generates > lots of contending printfs. Yes. I never got around to that, unfortunately. > Kernel printfs should be a very rare operation, and kernel syslog messages > not much more so, so concurrent printfs should be rare-squared. They > are a non-problem for me, so my old changes haven't been tested much. > But I needed serialization the other day for my printfs in trap() to > debug a deadlock in ddb, since the deadlock is related to to trap() > racing itself. I actually used PRINTF_BUFR_SIZE in a ~2008 kernel for > this. This is actually a good test of printf() under heavy load. Any > line buffering for printf() would have either broken the output format > (I wanted to show many checkpoint passes per line) or changed the timing > enough to make the race go away. > > My changes do the following to serialize per printf (not per line): > - try to acquire a nonstandard spinlock implemented using atomic_cmpset. > - spin for up to ~1 second (or more like 1 ms if you want) waiting for it > - ignore the lock after the timeout and proceeed (with complications to > not let multiple CPUs proceed at the same time, I hope) > - when multiple CPUs proceed, use atomic atomic ops as before to avoid > races in msgbuf accesses, and depend on console drivers being correct > to avoid deadlocks and races in them. Most aren't correct. > The worst that is supposed to happen is for deadlock on the nonstandard > spinlock to occur except for the timeout; then the timeout reduces to > interleaved output with bugs limited to console drivers by correct > msgbuf code. The deadlock on the nonstanded spinlock occurs in buggy > trap handlers for traps in code holding the lock (mainly ddb traps). > Then it mainly gives the amusing behaviour that the buggy code is > punished by printing its output very slowly. It sounds fine, but I don't have sufficient time to spend on this right now. So I can either back out the changes I mentioned above (assuming we get agreement from avg), or leave things as is. Ken -- Kenneth Merry ken@FreeBSD.ORG
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120217000846.GA7641>