From owner-freebsd-smp@FreeBSD.ORG Sun Sep 2 21:23:56 2007 Return-Path: Delivered-To: smp@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E8E8716A476 for ; Sun, 2 Sep 2007 21:23:56 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from nf-out-0910.google.com (nf-out-0910.google.com [64.233.182.185]) by mx1.freebsd.org (Postfix) with ESMTP id 8999613C4B6 for ; Sun, 2 Sep 2007 21:23:56 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: by nf-out-0910.google.com with SMTP id k4so1038846nfd for ; Sun, 02 Sep 2007 14:23:55 -0700 (PDT) DKIM-Signature: a=rsa-sha1; c=relaxed/relaxed; d=gmail.com; s=beta; h=domainkey-signature:received:received:message-id:date:from:sender:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references:x-google-sender-auth; b=GV1eGMeX5jUE+6b57qMQaTcRK+bznUOrqSbZ8UC4nQXejfQVpKThOEr8jA5hX9aB2mRpNC2TlLK8lT5MbdiCzu07tjt4wlDiYZY2Fkp1QCQY5CeWLurtmVb7sMZDtBxCDCvPzIAne8MF08Dg3ufO9gKovCk4LKYxYszLzGVkGvM= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:sender:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references:x-google-sender-auth; b=RPDVDn2XM/+Zx4aE/L7KEKvVH/QYO2aJM7BSiTCz8hIAGxe8Phq1SGG/4QoNK+JI3eEVKpv2B262dG2/LlSm/ZK3ydbgMmCizUW5zfy0UudPNUJtmaXKn1VAyJPcagi0FjaDzALX5mTNU6G4iosN9g3WyCsNVJmqXGyZqaLevSw= Received: by 10.78.172.20 with SMTP id u20mr2797838hue.1188768234738; Sun, 02 Sep 2007 14:23:54 -0700 (PDT) Received: by 10.78.97.18 with HTTP; Sun, 2 Sep 2007 14:23:54 -0700 (PDT) Message-ID: <3bbf2fe10709021423l62baee3ev5e490b07dd39ce7b@mail.gmail.com> Date: Sun, 2 Sep 2007 23:23:54 +0200 From: "Attilio Rao" Sender: asmrookie@gmail.com To: "Alfred Perlstein" In-Reply-To: <20070902210837.GP87451@elvis.mu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <20070831071048.GF87451@elvis.mu.org> <3bbf2fe10709011804r71dbde02wcb50b4b319476940@mail.gmail.com> <20070902210837.GP87451@elvis.mu.org> X-Google-Sender-Auth: fe7f2a9460a0e241 Cc: smp@freebsd.org Subject: Re: request for review: backport of sx and rwlocks from 7.0 to 6-stable X-BeenThere: freebsd-smp@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: FreeBSD SMP implementation group List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 Sep 2007 21:23:57 -0000 2007/9/2, Alfred Perlstein : > * Attilio Rao [070901 18:01] wrote: > > 2007/8/31, Alfred Perlstein : > > > 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