atomic_load_acq_int in sequential_heuristic

[Konstantin Belousov]

On Sun, Aug 24, 2014 at 01:57:29PM +0200, Mateusz Guzik wrote:
> 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.
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.
> 
> 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>
> _______________________________________________
> freebsd-hackers at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at freebsd.org"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20140824/6f165dbc/attachment.sig>