Date: Sun, 2 Sep 2007 23:23:54 +0200 From: "Attilio Rao" <attilio@freebsd.org> To: "Alfred Perlstein" <alfred@freebsd.org> Cc: smp@freebsd.org Subject: Re: request for review: backport of sx and rwlocks from 7.0 to 6-stable Message-ID: <3bbf2fe10709021423l62baee3ev5e490b07dd39ce7b@mail.gmail.com> In-Reply-To: <20070902210837.GP87451@elvis.mu.org> References: <20070831071048.GF87451@elvis.mu.org> <3bbf2fe10709011804r71dbde02wcb50b4b319476940@mail.gmail.com> <20070902210837.GP87451@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
2007/9/2, Alfred Perlstein <alfred@freebsd.org>: > * Attilio Rao <attilio@freebsd.org> [070901 18:01] wrote: > > 2007/8/31, Alfred Perlstein <alfred@freebsd.org>: > > > Hi guys, > > > > > > Some work here at work was approved for sharing with community so > > > I'm posting it here in hope of a review. > > > > > > We run some pretty good stress testing on our code, so I think it's > > > pretty solid. > > > > > > My only concern is that I've tried my best to preserve kernel source > > > API, but not binary compat though a few simple #defines. > > > > > > I can make binary compat, in albeit a somewhat confusing manner, but > > > that will require some rototilling and weird renaming of calls to > > > the sleepq and turnstile code. In short, I'd rather not, but I will > > > if you think it's something that should be done. > > > > > > There's also a few placeholders for lock profiling which I will > > > very likely be backporting shortly as well. > > > > > > Patch is attached. > > > > > > Comments/questions? > > > > Hello Alfred, > > I started looking at the patch and I have 2 things to say: > > - why you backported the allocating patch with UMA in sleepqueues? it > > is ortogonhal to this problem and it is not necessary due in this case > > Well, it has performance implications so I took it. > > > I think > > - Instead than using the stub __aligned() for struct thread, you > > should use what we alredy do for 7.0 as dealing with uma allocation > > functions and a separate stub for thread0. you can workaround the > > missing of uma functions with a simple macro. > > > > I will try to give a line-by-line revision ASAP. > > I don't really agree here. I think that since we rely on > the object to be aligned such that otherwise bad and hard to track things > happen, putting the alignment declaration in the variable rather > than structure declaration will lead to an easy to avoid problem > that is nearly impossible to track down. > > Ie. if someone makes thread1, then everyone with non default > 16 byte aligned platforms will break unless they are smart > enough to see thread0 and understand. > > I guess the question is, why NOT force the alignment at the > structure declaration other than asthetics? This is not what I meant. What I was suggesting is to use uma_zcreate() to handle alignment of struct thread as we do for 7.0. thread0 is a special case, and it needs to be handled separately and it requires to use __aligned() in its definition. But it is alone. Thanks, Attilio -- Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3bbf2fe10709021423l62baee3ev5e490b07dd39ce7b>