request for review: backport of sx and rwlocks from 7.0 to 6-stable

[Attilio Rao]

2007/9/2, Alfred Perlstein <alfred at freebsd.org>:
> * Attilio Rao <attilio at freebsd.org> [070901 18:01] wrote:
> > 2007/8/31, Alfred Perlstein <alfred at 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