[OpenAFS] Future Bugs in ./src/util/uuid.c

John Hascall john@iastate.edu
Thu, 05 Oct 2006 12:04:14 CDT


I happend to be looking in uuid.c and noticed
that should there ever be a machine fast enough
to create 32768 UUIDs per clock tick then once
afs_uuid_create() sets got_no_time to 1 it never
unsets it, which would result in:
  do {
       ...
  } while (got_no_time);
looping forever.

The fastest machine I found could do about 900 per tick.
(For machines which twiddle with the low order bits
returned by multiple gettimeofday calls per clock tick,
this is probably a non-issue, FWIW.)

I have enclosed a patch diff below.

Also, if a machine can do (10,000,000/TicksPerSec)
calls to afs_uuid_create() in a single tick it seems to me
that a duplicate uuid could be created.

For example, the machine I tested on ticks at TicksPerSec = 1024,
so that means the granularity of timeval.tv_usec is 976.
This value is multiplied by 10 to make the low order clock bits of the uuid.
Multiple calls inside a tick increment this value, so if you could
do 9760 calls (about 11x faster) you could have duplicates.

So, ideally instead of using the fixed value 0x7fff for
the limit of uuid_time_adjust it should be such that you
can't create more than 10x the minimum tv_usec delta
(10,000,000/TicksPerSec for systems that don't twiddle tv_usec).
No patch provided for this, but one possible approach would be
add:
   static int adjust_lim;
and inside of the
   if (!uuid_init_done) { ... }
block to replace:
   uuid__get_os_time(&time_last);
with something like:
   for (code = 1; code != -1; code = time_cmp(&time_now, &time_last)) {
      if (code == 1) uuid__get_os_time(&time_now);
      uuid__get_os_time(&time_last);
   }
   if ((adjust_lim = (time_last.lo - time_now.lo)) < 0) adjust_lim += 1000000;
and replace
   0x7fff
with
   adjust_lim

I have no idea how often afs calls afs_uuid_create(),
perhaps it is infrequent enough that this is totally
unlikely to happen in "real life".

And finally, because you lop off the top 4 bits of tv_sec
you have the possibility of recycling to a previously issued
uuid if you are up more than about 8.5 years.

John
------------------- begin patch diff -----------------
127c127
<  * |            node ID               |  8-16           .node
---
>  * |            node ID               |  10-16          .node
255c255
<     afs_int32 got_no_time = 0, code;
---
>     afs_int32 code;
292a293
> 	uuid_time_adjust = 0;
318,320c319
< 	    if (uuid_time_adjust == 0x7fff)	/* spin while we wait for the clock to tick */
< 		got_no_time = 1;
< 	    else
---
> 	    if (uuid_time_adjust != 0x7fff)	/* if not max */
323c322
<     } while (got_no_time);
---
>     } while (uuid_time_adjust == 0x7fff);	/* spin until clock tick */
------------------- end patch diff -----------------