Date: Sat, 16 Aug 2014 12:38:11 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Mateusz Guzik <mjguzik@gmail.com> Cc: Konstantin Belousov <kib@FreeBSD.org>, Johan Schuijt <johan@transip.nl>, freebsd-arch@freebsd.org Subject: Re: [PATCH 1/2] Implement simple sequence counters with memory barriers. Message-ID: <20140816093811.GX2737@kib.kiev.ua> In-Reply-To: <1408064112-573-2-git-send-email-mjguzik@gmail.com> References: <1408064112-573-1-git-send-email-mjguzik@gmail.com> <1408064112-573-2-git-send-email-mjguzik@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--BkCcXYe/IK2urb/q
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Fri, Aug 15, 2014 at 02:55:11AM +0200, Mateusz Guzik wrote:
> ---
> sys/sys/seq.h | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++=
++++++
> 1 file changed, 126 insertions(+)
> create mode 100644 sys/sys/seq.h
>=20
> diff --git a/sys/sys/seq.h b/sys/sys/seq.h
> new file mode 100644
> index 0000000..0971aef
> --- /dev/null
> +++ b/sys/sys/seq.h
> @@ -0,0 +1,126 @@
> +/*-
> + * Copyright (c) 2014 The FreeBSD Project
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distributio=
n.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PU=
RPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIAB=
LE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUE=
NTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOO=
DS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, S=
TRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY=
WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + *
> + * $FreeBSD$
> + */
> +
> +#ifndef _SYS_SEQ_H_
> +#define _SYS_SEQ_H_
> +
> +#ifdef _KERNEL
> +
> +/*
> + * Typical usage:
> + *
> + * writers:
> + * lock_exclusive(&obj->lock);
> + * seq_write_begin(&obj->seq);
> + * .....
> + * seq_write_end(&obj->seq);
> + * unlock_exclusive(&obj->unlock);
> + *
> + * readers:
> + * obj_t lobj;
> + * seq_t seq;
> + *
> + * for (;;) {
> + * seq =3D seq_read(&gobj->seq);
> + * lobj =3D gobj;
> + * if (seq_consistent(&gobj->seq, seq))
> + * break;
> + * cpu_spinwait();
> + * }
> + * foo(lobj);
> + */ =09
> +
> +typedef uint32_t seq_t;
> +
> +/* A hack to get MPASS macro */
> +#include <sys/systm.h>
> +#include <sys/lock.h>
> +
> +#include <machine/cpu.h>
> +
> +static __inline bool
> +seq_in_modify(seq_t seqp)
> +{
> +
> + return (seqp & 1);
> +}
> +
> +static __inline void
> +seq_write_begin(seq_t *seqp)
> +{
> +
> + MPASS(!seq_in_modify(*seqp));
> + (*seqp)++;
> + wmb();
This probably ought to be written as atomic_add_rel_int(seqp, 1);
Same note for all other linux-style barriers. In fact, on x86
wmb() is sfence and it serves no useful purpose in seq_write*.
Overall, it feels too alien and linux-ish for my taste.
Since we have sequence bound to some lock anyway, could we introduce
some sort of generation-aware locks variants, which extend existing
locks, and where lock/unlock bump generation number ?
> +}
> +
> +static __inline void
> +seq_write_end(seq_t *seqp)
> +{
> +
> + wmb();
> + (*seqp)++;
> + MPASS(!seq_in_modify(*seqp));
> +}
> +
> +static __inline seq_t
> +seq_read(seq_t *seqp)
> +{
> + seq_t ret;
> +
> + for (;;) {
> + ret =3D READ_ONCE(*seqp);
> + if (seq_in_modify(ret)) {
> + cpu_spinwait();
> + continue;
> + }
> + break;
> + }
> +
> + rmb();
> +
> + return (ret);
> +}
> +
> +static __inline seq_t
> +seq_consistent_nomb(seq_t *seqp, seq_t oldseqp)
> +{
> +
> + MPASS(!seq_in_modify(oldseqp));
> + return (*seqp =3D=3D oldseqp);
> +}
> +
> +static __inline seq_t
> +seq_consistent(seq_t *seqp, seq_t oldseqp)
> +{
> +
> + rmb();
> + return (seq_consistent_nomb(seqp, oldseqp));
> +}
> +
> +#endif /* _KERNEL */
> +#endif /* _SYS_SEQ_H_ */
> --=20
> 2.0.2
--BkCcXYe/IK2urb/q
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBAgAGBQJT7yaDAAoJEJDCuSvBvK1BTRMP/1ECsFOhc06qSXshkUgScpQ7
xbAghb6WYTI/I4Es3w9aBq6AxK1YfaxjW0Liv0/YFAkkUnSU9Ko6xpoY2QZDBkv/
5BxCA8Qd5F02GDz7lnPt2iiCevDJXmGQqEtt3VKzE26eoinJX38v1lKvFSTMAXKq
UXZ9YIsL5TrLUECoM+UuB5CtPbD7/TtQ2W6XBgqCKYGxjy9zWHysRWi8sej0kwst
3zzztnCaRGSNwDhnaKfZND21QVzmMx6budSi6Q5/UjBiFrIBXtJ+Ioq7bc+tmiOO
dTUq2BFyhkN+LgqSLK/+2X8dT0qemwrJWzuAw2tcxzx6Wdq0zPAmwtZgyPar+rZh
fdrbXnSHXhMaBaqZJsVyp6BRZ+TfSRNQ+wWaYdFcsme9OTMe7ciZCxaCKCYho8dX
yE38GOmziBWRqu0LbL1fhQuc1XE5RmaMiUR6MEyfa/t2/RjLJJEVJ3ec+1esLB0W
CdwrXeObDMMqE/IWxVIteEWOPI88r/RkNcjnLOm6aJ4JGvvqiE+ugVlsIsLZI2Qp
P+Cq8+afDsQRWbwi6jZjESx1W3XecwhAcLvjLpu4KXICi8QJSCwxfMjU9pUR0weD
npK1C70skOqJCi4Ty7oDHrmmUAt6NueMMIbESBfbwae/CnrQSHdii4Oc1s5X/G2+
yQmbWjUPT6CLXaLfv5fH
=LoS0
-----END PGP SIGNATURE-----
--BkCcXYe/IK2urb/q--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140816093811.GX2737>