Date: Sat, 02 Jun 2007 20:53:18 +0200 From: Attilio Rao <attilio@FreeBSD.org> To: Rui Paulo <rpaulo@FreeBSD.org> Cc: Perforce Change Reviews <perforce@FreeBSD.org> Subject: Re: PERFORCE change 120788 for review Message-ID: <4661BC9E.9070603@FreeBSD.org> In-Reply-To: <200706021756.l52Huq9A049371@repoman.freebsd.org>
index | next in thread | previous in thread | raw e-mail
Rui Paulo wrote: > http://perforce.freebsd.org/chv.cgi?CH=120788 > > Change 120788 by rpaulo@rpaulo_epsilon on 2007/06/02 17:55:58 > > Add locking. > > Affected files ... > > .. //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmc.c#8 edit > .. //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmcvar.h#4 edit > > Differences ... > > ==== //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmc.c#8 (text+ko) ==== > > @@ -23,7 +23,7 @@ > * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE > * POSSIBILITY OF SUCH DAMAGE. > * > - * $P4: //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmc.c#7 $ > + * $P4: //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmc.c#8 $ > * > */ > > @@ -46,7 +46,8 @@ > #include <sys/conf.h> > #include <sys/kernel.h> > #include <sys/sysctl.h> > -#include <sys/sbuf.h> > +#include <sys/lock.h> > +#include <sys/mutex.h> > > #include <isa/isavar.h> You should mantain includes sorted by name (where possible). > @@ -239,6 +240,8 @@ > > model = asmc_match(dev); > > + mtx_init(&sc->sc_mtx, "asmc_mtx", NULL, MTX_SPIN); > + > asmc_init(dev); You don't need to write again 'mtx' in the description. Just add something descriptive for the mutexes (since you alredy know it is a spin mutexes). > @@ -740,6 +756,9 @@ > { > uint8_t type; > device_t dev = (device_t) arg; > + struct asmc_softc *sc = device_get_softc(dev); > + > + mtx_lock_spin(&sc->sc_mtx); > > type = inb(ASMC_INTPORT); > > @@ -757,6 +776,8 @@ > device_printf(dev, "%s unknown interrupt\n", __func__); > } > > + mtx_unlock_spin(&sc->sc_mtx); I'm not sure you really need to maintain the lock for so long. Probabilly, you just need for the duration of inb(). > ==== //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmcvar.h#4 (text+ko) ==== > > @@ -23,7 +23,7 @@ > * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE > * POSSIBILITY OF SUCH DAMAGE. > * > - * $P4: //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmcvar.h#3 $ > + * $P4: //depot/projects/soc2007/rpaulo-macbook/dev/asmc/asmcvar.h#4 $ > * > */ > > @@ -49,6 +49,8 @@ > int sc_rid; > struct resource *sc_res; > void *sc_cookie; > + > + struct mtx sc_mtx; > }; The extra white-line is not needed and since mutexes are not so light, it would be better to put it on the top of the structure. Attiliohome | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4661BC9E.9070603>