Re: [PATCH net V4 2/2] igb: offer a PTP Hardware Clock instead of the timecompare method

Jacob,

Thanks for your detailed explanation...

On Mon, Mar 26, 2012 at 08:46:13PM +0000, Keller, Jacob E wrote:

The issue with gettime is in the SYSTIM registers. Gettime could also
use cyc2time, and allow the background worktask to be the only one updating
the timecounter's last read value. (this only has to happen at least twice
every wraparound value, so for the 10gig 82599 about once every 30 seconds).
But the lock is also protecting against corruption of the SYSTIM register
reads. (The lock does 2 different things. We could add a separate lock,
but I am not convinced it is worth it).

Right, the spinlock in my patch protects both the SYSTIM registers and
the struct timecounter. For the SYSTIM pair, you must use a
spinlock. There is no way around it.

The struct timecounter fields .cycle_last and .nsec are used as follows

   WRITE:   timercoutner_read()
            timercoutner_init()

   READ:    timercoutner_cyc2time()

Here you could conceivably use a reader/writer semaphore or a
seqlock. That would in turn enlarge the data structure and would
require nesting the locks.

I really doubt you will see any performance gain from such a
change. It increases code complexity and size for some dubious,
theoretical performance gain, for some really whacked use case.

Richard