svn commit: r241889 - in user/andre/tcp_workqueue/sys: arm/arm cddl/compat/opensolaris/kern cddl/contrib/opensolaris/uts/common/dtrace cddl/contrib/opensolaris/uts/common/fs/zfs ddb dev/acpica dev/...

John Baldwin jhb at freebsd.org
Tue Oct 23 14:18:51 UTC 2012


On Tuesday, October 23, 2012 5:35:03 am Andre Oppermann wrote:
> On 22.10.2012 19:10, Bruce Evans wrote:
> > On Mon, 22 Oct 2012, Andre Oppermann wrote:
> >
> >> Log:
> >>  Second pass at making global mutexes use MTX_DEF_SYSINIT().
> >
> >> Modified: user/andre/tcp_workqueue/sys/arm/arm/busdma_machdep.c
> >> ==============================================================================
> >> --- user/andre/tcp_workqueue/sys/arm/arm/busdma_machdep.c    Mon Oct 22 14:10:17 2012    (r241888)
> >> +++ user/andre/tcp_workqueue/sys/arm/arm/busdma_machdep.c    Mon Oct 22 14:18:22 2012    (r241889)
> >> @@ -163,9 +163,7 @@ static TAILQ_HEAD(,bus_dmamap) dmamap_fr
> >> #define BUSDMA_STATIC_MAPS    500
> >> static struct bus_dmamap map_pool[BUSDMA_STATIC_MAPS];
> >>
> >> -static struct mtx busdma_mtx;
> >> -
> >> -MTX_SYSINIT(busdma_mtx, &busdma_mtx, "busdma lock", MTX_DEF);
> >> +static MTX_DEF_SYSINIT(busdma_mtx, "busdma lock", MTX_DEF);
> >
> > The verboser (sic) name is uglier.
> >
> > But once it says DEF, the arg list shouldn't say DEF.  Either this is
> > DEF for something unrelated to MTX_DEF, in which case it is confusing
> > and the confusion is made worse by having MTX_ precede DEF in both,
> > or it is the same DEF and the arg is redundant, or it implies MTX_DEF
> > by default but this can be changed, in which case it is confusing.
> 
> You're right, it is a bit confusing.  However the DEF in MTX_DEF_SYSINIT
> stands for DEFining the global variable whereas MTX_DEF stands for a
> default type mutex.
> 
> Unfortunately I couldn't come up with a bette short name for MTX_DEF_SYSINIT.
> I'm open for suggestions though.

It's not super clear to me that having one macro to replace
'struct mtx and MTX_SYSINIT' is substantially more readable compared to just
adding '__aligned(CACHE_LINE_SIZE)' at the end of global locks.

Note, btw, that I have local patches to make mtx_pool's use aligned mutexes,
but they have never shown a performance improvement in real-word benchmarks
(granted, the last set of benchmarks were run several years ago (the patches
are old) by kris@).  OTOH, alc@ found a measurable boost by aligning a few
global mutexes in the VM system.

-- 
John Baldwin


More information about the svn-src-user mailing list