Re: [PATCH] optimize writer path in time_interpolator_get_counter()

[tony.luck@intel.com]

+			/* When holding the xtime write lock, there's no need
+			 * to add the overhead of the cmpxchg.  Readers are
+			 * force to retry until the write lock is released.
+			 */
+			if (writelock) {
+				time_interpolator->last_cycle = now;
+				return now;
+			}

This is lots prettier that my first suggestion (no surprise, even _I_ didn't like
my suggested patch :-)

In theory it looks like it should help a lot as it ought to prevent the worst case
spinning when one cpu has the xtime write lock.

Sadly, running my test case (running 1-4 tasks, each bound to a cpu, each pounding
on gettimeofday(2)) I'm still seeing significant time spent spinning in this loop.
Things are better: worst case time was down to just over 2ms from 34ms ... which
is a significant improvement ... but 2ms still sounds ugly.

I'm still seeing the asymmetric behavior where cpu3 sees the really high times,
while cpu0,1,2 are seeing peaks of 170us, which is still not pretty.

So this is a very half-hearted ACK for the patch.  It does improve things, but I
still think that code that does:

	do {
		val = xxx;

		// execute some stuff here
	} while (cmpxchg(&xxx, val, newval) != val);

is badly flawed.  If this were in some rare code path that was only likely to see
collisions when more than three planets were aligned, then it might be OK, but this
is a common path and it is easy for (malicious) users to force multiple cpus into
running this code ... making it likely that the cmpxchg will fail repeatedly.

-Tony

P.S. My ugly patch does even less to fix this problem ... times are worse than this.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/