Date: Fri, 8 Aug 2008 10:08:59 -0400 From: John Baldwin <jhb@freebsd.org> To: freebsd-current@freebsd.org Cc: current@freebsd.org, pjd@freebsd.org, Patrick =?iso-8859-15?q?Lamaizi=E8re?= <patfbsd@davenulle.org>, Mike Tancsa <mike@sentex.net> Subject: Re: AMD Geode LX crypto accelerator (glxsb) (and padlock.ko) Message-ID: <200808081008.59874.jhb@freebsd.org> In-Reply-To: <20080807235854.1812d866@baby-jane.lamaiziere.net.home> References: <20080606234135.46144207@baby-jane-lamaiziere-net.local> <200808072045.m77KjaGO098913@lava.sentex.ca> <20080807235854.1812d866@baby-jane.lamaiziere.net.home>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday 07 August 2008 05:58:54 pm Patrick Lamaizi=E8re wrote: > Le Thu, 07 Aug 2008 16:45:37 -0400, > > Mike Tancsa <mike@sentex.net> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200808081008.59874.jhb>