From owner-freebsd-hackers@FreeBSD.ORG Mon May 10 15:35:45 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 202291065670 for ; Mon, 10 May 2010 15:35:45 +0000 (UTC) (envelope-from sam@errno.com) Received: from ebb.errno.com (ebb.errno.com [69.12.149.25]) by mx1.freebsd.org (Postfix) with ESMTP id EA7038FC15 for ; Mon, 10 May 2010 15:35:44 +0000 (UTC) Received: from [172.24.98.37] ([192.75.139.252]) (authenticated bits=0) by ebb.errno.com (8.13.6/8.12.6) with ESMTP id o4AFQ9fr056289 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Mon, 10 May 2010 08:26:11 -0700 (PDT) (envelope-from sam@errno.com) Message-Id: From: Sam Leffler To: Patrick Lamaiziere In-Reply-To: <20100510001637.2a564cd5@davenulle.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed; delsp=yes Content-Transfer-Encoding: quoted-printable Mime-Version: 1.0 (Apple Message framework v936) Date: Mon, 10 May 2010 08:26:08 -0700 References: <4BE46650.7010008@bluezbox.com> <20100510001637.2a564cd5@davenulle.org> X-Mailer: Apple Mail (2.936) X-DCC--Metrics: ebb.errno.com; whitelist Cc: freebsd-hackers@freebsd.org Subject: Re: hifn(4) DMA fix for review X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 May 2010 15:35:45 -0000 On May 9, 2010, at 3:16 PM, Patrick Lamaiziere wrote: > Le Fri, 07 May 2010 12:13:20 -0700, > Oleksandr Tymoshenko a =E9crit : > > Hi, > >> Proposed patch addresses hifn(4) problems on FreeBSD/mips. >> Current implementation keeps some of the state information (indexes =20= >> in >> buffers, etc) in DMA-mapped memory and bus_dma code invalidates them >> during sync operations. This fix moves data that doesn't belong to =20= >> DMA >> ring to softc structure. > > I do not have any comment but I will try on my Soekris (the next > weekend) if it can help. > > I noticed several things in hifn, if you want to look: > http://www.freebsd.org/cgi/query-pr.cgi?pr=3Dkern/130286 > > IMHO some locks are missing in the use of sc->sc_sessions (the array > containing the sessions) : in hifn_newsession(), if there is no space > left in sc->sc_sessions, a new array is allocated and the sessions are > copied to it. Unfortunaly, sc->sc_sessions is used in hifn_process > without any lock and we use some pointers on the array (my patch =20 > should > addresses this if I remember...). Isn't this just the glx locking? (no offense intended) I've said =20 before I think we to move session management up into the crypto layer =20= since it's implemented in many drivers (usually w/ c&p of the same =20 code as you noted here sometimes a bit different). Sam