Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 24 Aug 2014 19:23:31 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Mateusz Guzik <mjguzik@gmail.com>, freebsd-hackers@freebsd.org
Subject:   Re: atomic_load_acq_int in sequential_heuristic
Message-ID:  <20140824162331.GW2737@kib.kiev.ua>
In-Reply-To: <20140824115729.GC2045@dft-labs.eu>
References:  <20140824115729.GC2045@dft-labs.eu>

next in thread | previous in thread | raw e-mail | index | archive | help

--3v3azX03nJMP4RYa
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Aug 24, 2014 at 01:57:29PM +0200, Mateusz Guzik wrote:
> Writer side is:
> fp->f_seqcount =3D (arg + bsize - 1) / bsize;
> do {
> 	new =3D old =3D fp->f_flag;
> 	new |=3D FRDAHEAD;
> } while (!atomic_cmpset_rel_int(&fp->f_flag, old, new));
>=20
> Reader side is:
> if (atomic_load_acq_int(&(fp->f_flag)) & FRDAHEAD)
> 	return (fp->f_seqcount << IO_SEQSHIFT);
>=20
> We can easily get the following despite load_acq:
> CPU0				CPU1
> 				load_acq fp->f_flag
> fp->f_seqcount =3D ...
> store_rel fp->f_flag
> 				read fp->f_seqcount
> 			=09
> So the barrier does not seem to serve any purpose.
It does.

Consider initial situation, when the flag is not set yet. There, we
do not want to allow the reader to interpret automatically calculated
f_seqcount as the user-supplied constant.  Without barriers, we might
read the flag as set, while user-provided value for f_seqcount is still
not visible to processor doing read.

> Given that this is only a hint and rarely changes it should be fine.
>=20
> So how about the following:
> diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c
> index f1d19ac..2e056de 100644
> --- a/sys/kern/vfs_vnops.c
> +++ b/sys/kern/vfs_vnops.c
> @@ -438,7 +438,7 @@ static int
>  sequential_heuristic(struct uio *uio, struct file *fp)
>  {
> =20
> -       if (atomic_load_acq_int(&(fp->f_flag)) & FRDAHEAD)
> +       if (fp->f_flag & FRDAHEAD)
>                 return (fp->f_seqcount << IO_SEQSHIFT);
> =20
>         /*
>=20
> I make no claims about any performance difference in this one. This is
> only a cleanup.
>=20
> --=20
> Mateusz Guzik <mjguzik gmail.com>
> _______________________________________________
> freebsd-hackers@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"

--3v3azX03nJMP4RYa
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJT+hGDAAoJEJDCuSvBvK1BdvEP/37QxMM4WZclxuyXGsHNgyjX
TIZRJgZYGhUxzXF7BlEGKbSjE9PQo2BQrm9TTGD7JBJw434Ufa2SZ36JwdtY9Z9t
YHtRCN8bX68OygrKvJrw6sgZkUKVpWPiUr7dUbru/KXiqAtxnldO42os1fjayxsJ
0eecsikNtkG1weMLU4LeK9l+7k6bzg/caGiYcnzzghja5O9C+Zu7bmB7CfSOlTZQ
DMa7LCBqbL6NdGVt27gyELr87McNhNbMsfxlOiTOH4wEgC1s9f5qGMTqnU1pO/sT
obdoDlxYJegSXmDvEu/6Yy2SxI+M/HiuDkB81OZUvfeeJSGIH/azjjZUblJFxiG/
N5ZO0fuLABgVbgI/jQ2NWxgvASHDr11wDKbD7WcfWwrPYaS7b9VfMHMmP8UwO8j2
a23XijINjDYUmWuhKjSTTuGO/ap1AO/upEuL+OZp7t4Ym69UA/KrNISnRJQlVuju
NkyWRt6Y49gDzIbRFFwgkNVocIu4VKWQua4W1+cdpNigjvqi6a+1r4zvXQwvYZoA
n21MHdPUFzxPeYi1vgSV4tJR55GCio707qljucS1M10++uEUbuylOTff6FZ5qTRk
EXgkPzz3F/zy/RNrrztYhGvcwODiM0ssHDIMEfQq9VfSwtj4Z9kBXaNb7N/bFQ8+
a/8l9VAqAfcGY0/LR9UJ
=dt2I
-----END PGP SIGNATURE-----

--3v3azX03nJMP4RYa--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140824162331.GW2737>