From owner-freebsd-current@FreeBSD.ORG Fri Aug 8 14:10:18 2008 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 925231065679; Fri, 8 Aug 2008 14:10:18 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 23E7E8FC23; Fri, 8 Aug 2008 14:10:18 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from zion.baldwin.cx (zion.baldwin.cx [IPv6:2001:470:1f11:75:2a0:d2ff:fe18:8b38]) (authenticated bits=0) by server.baldwin.cx (8.14.2/8.14.2) with ESMTP id m78EA9pf028840; Fri, 8 Aug 2008 10:10:09 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: freebsd-current@freebsd.org Date: Fri, 8 Aug 2008 10:08:59 -0400 User-Agent: KMail/1.9.7 References: <20080606234135.46144207@baby-jane-lamaiziere-net.local> <200808072045.m77KjaGO098913@lava.sentex.ca> <20080807235854.1812d866@baby-jane.lamaiziere.net.home> In-Reply-To: <20080807235854.1812d866@baby-jane.lamaiziere.net.home> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Message-Id: <200808081008.59874.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [IPv6:2001:470:1f11:75::1]); Fri, 08 Aug 2008 10:10:09 -0400 (EDT) X-Virus-Scanned: ClamAV 0.93.1/7978/Fri Aug 8 08:33:49 2008 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00,NO_RELAYS autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: current@freebsd.org, pjd@freebsd.org, Patrick =?iso-8859-15?q?Lamaizi=E8re?= , Mike Tancsa Subject: Re: AMD Geode LX crypto accelerator (glxsb) (and padlock.ko) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Aug 2008 14:10:18 -0000 On Thursday 07 August 2008 05:58:54 pm Patrick Lamaizi=E8re wrote: > Le Thu, 07 Aug 2008 16:45:37 -0400, > > Mike Tancsa a =E9crit : > > At 10:34 PM 8/6/2008, Mike Tancsa wrote: > > >At 05:42 PM 7/6/2008, Patrick Lamaizi=E8re wrote: > > >>I think that the driver is ready and works fine, i didn't change > > >>anything since the latest version "22/06/08" > > >>http://user.lamaiziere.net/patrick/glxsb-220608.tar.gz > > > > > >Hi, > > > I was doing some testing with openvpn > > > and was wondering if there is perhaps a memory leak ? > > Yes, padlock and glxsb may not reuse the free sessions. See: > http://www.nabble.com/Recent-Padlock-changes-break-ssh-tt18582674.html#a1= 85 >82674 > > For glxsb, i've made a diff between current that include a fix: > http://user.lamaiziere.net/patrick/diff-glxsb-8.txt > > Thank you, regards. A few suggestions of things I noticed in the patch: =2D Use __RCSID("$FreeBSD"); in place of the KERNEL_RCSID stuff. =2D You shouldn't call pci_enable_io(). bus_alloc_resource() will do that for you after it has programmed the BAR. =2D Don't use bus space tag/handle. Instead of bus_space_foo(tag, handle, = =2E..) use bus_foo(resource, ...) =2D Functions w/o any local variables should start with a blank line before the code (style(9)), e.g. the probe() routine. =2D Less whitespace in certain areas. For example, I would reduce the two lines after setting the bus tag/handle down to 1, and I would remove the blank lines after the "Disable interrupts" and "Initialize our task queue" comments. =2D No need to explicitly zero the softc in attach(), new-bus already does = this for you. =2D Just use 'device_set_desc()' in probe. device_set_desc_copy() is for w= hen you are working with a string that will go away (such as if you generate a description into a buffer on the stack). =2D You need to do a taskqueue_drain() in your detach routine before taskqueue_free(), but after detaching the driver from the crypto interface and anything else that could schedule the task to run. Probably fine to do this either right before or right after the current call to callout_stop(). =2D You should do a callout_drain() rather than a callout_stop() in your de= tach routine. =2D-=20 John Baldwin