Date: Tue, 11 May 2010 19:11:40 +0200 From: Patrick Lamaiziere <patfbsd@davenulle.org> To: Sam Leffler <sam@errno.com> Cc: freebsd-hackers@freebsd.org Subject: Re: hifn(4) DMA fix for review Message-ID: <20100511191140.2111cc70@davenulle.org> In-Reply-To: <B937FBA2-4967-4977-BDEA-0B46D5A3E0F5@errno.com> References: <4BE46650.7010008@bluezbox.com> <20100510001637.2a564cd5@davenulle.org> <B937FBA2-4967-4977-BDEA-0B46D5A3E0F5@errno.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Le Mon, 10 May 2010 08:26:08 -0700, Sam Leffler <sam@errno.com> a écrit : Hello, > > I noticed several things in hifn, if you want to look: > > http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/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 should > > addresses this if I remember...). > > Isn't this just the glx locking? (no offense intended) I've said > before I think we to move session management up into the crypto > layer since it's implemented in many drivers (usually w/ c&p of the > same code as you noted here sometimes a bit different). Yes, the code comes mostly from padlock and glxsb. glxsb and padlock use a TAILQ to store the sessions instead of an array. It is better when we want to add or remove a session but we have to search the session each time when processing the operation. In hifn, the session id (sid) is directly the index so it should be more efficient. I'm not sure if this is a concern or not. Regards.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100511191140.2111cc70>