Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 24 Aug 2014 13:57:29 +0200
From:      Mateusz Guzik <mjguzik@gmail.com>
To:        freebsd-hackers@freebsd.org
Subject:   atomic_load_acq_int in sequential_heuristic
Message-ID:  <20140824115729.GC2045@dft-labs.eu>

next in thread | raw e-mail | index | archive | help
Writer side is:
fp->f_seqcount = (arg + bsize - 1) / bsize;
do {
	new = old = fp->f_flag;
	new |= FRDAHEAD;
} while (!atomic_cmpset_rel_int(&fp->f_flag, old, new));

Reader side is:
if (atomic_load_acq_int(&(fp->f_flag)) & FRDAHEAD)
	return (fp->f_seqcount << IO_SEQSHIFT);

We can easily get the following despite load_acq:
CPU0				CPU1
				load_acq fp->f_flag
fp->f_seqcount = ...
store_rel fp->f_flag
				read fp->f_seqcount
				
So the barrier does not seem to serve any purpose.
Given that this is only a hint and rarely changes it should be fine.

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)
 {
 
-       if (atomic_load_acq_int(&(fp->f_flag)) & FRDAHEAD)
+       if (fp->f_flag & FRDAHEAD)
                return (fp->f_seqcount << IO_SEQSHIFT);
 
        /*

I make no claims about any performance difference in this one. This is
only a cleanup.

-- 
Mateusz Guzik <mjguzik gmail.com>



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