atomic_load_acq_int in sequential_heuristic

[Konstantin Belousov]

On Sun, Aug 24, 2014 at 07:23:31PM +0300, Konstantin Belousov wrote:
> 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.
That said, I think now that there is a real bug.

If we did not read the FRDAHEAD in sequential_heuristic(), we may
override user-supplied value for f_seqcount.  I do not see other
solution than start to use locking.
-------------- 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/d5f94d46/attachment.sig>