On Thu, Apr 02, 2015 at 07:58:29AM -0600, Ian Lepore wrote: > On Thu, 2015-04-02 at 15:51 +0200, Mateusz Guzik wrote: > > On Thu, Apr 02, 2015 at 04:42:17PM +0300, Gleb Smirnoff wrote: > > > On Thu, Apr 02, 2015 at 03:37:51PM +0200, Mateusz Guzik wrote: > > > M> For this particular use-case you never care what CPU you are executing > > > M> on, you only want to obtain a unique number. > > > M> > > > M> per-cpu counters can serve this purpose no problem, just provide an > > > M> operation which guarantees to return the new value of the counter it > > > M> incremented. Should be easily achieved with e.g. just pinning curthread > > > M> to the cpu it executes on for the duration of inc + fetch. > > > > > > I'd ask to pay attention to this particular email: > > > > > > https://lists.freebsd.org/pipermail/svn-src-head/2015-March/069966.html > > > > > > Just to justify probabilities, risks and countermeasures. > > > > > > For those, who don't believe in theory and prefers practice: > > > > > > https://lists.freebsd.org/pipermail/svn-src-head/2015-March/070091.html > > > > > > Note that Emeric was the one who observed collisions for the ip_id++ > > > code, that we used before. > > > > > > > Well in that case this at least deserves a comment in the code. Everyone > > spotting that counter_u64_add + zpcpu_get will think it's a bug. > > > > Because it IS a bug. That isn't changed by the fact that it works > reliably on one platform due to what should be an opque implementation > detail, and works by accident on other platforms (at least until the > details of their implementations change in the future). > > As soon as somebody sees this code, thinks it's a good way to do things, > and cut and pastes it into another venue and removes the & 0xffff, it > just turns into a bug on every platform except amd64. >
Well, a thread migrating to another cpu is the standard thing everyone sees. Are you referring to the fact that the counter is 64-bit, so 32-bit arches must perform 2 reads and the thread can be migrated in-between? Indeed that would look like a solid problem, mitigated 'by accident' with 0xffff. I don't feel that strongly about changing the code. If it stays as it is, it definitely needs the comment I mentioned explaining why migration is fine. If the code was to be changed a machdep counter_u64_fetchadd seems like the only course of action. -- Mateusz Guzik <mjguzik gmail.com> _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
