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

On Thu, Mar 22, 2012 at 2:41 AM, Richard Cochran
[off-list ref] wrote:

On Wed, Mar 21, 2012 at 05:50:34PM -0400, chetan loke wrote:

quoted

Richard - Intent is to make the readers(get_time) wait (or return last
read value if the seq_counter tripped because you know that this value
was recent) and let the tx/rx path continue. I haven't looked in more
details but as Jake mentioned you will also need to change the way you
read the values(by not using timecounter_read in get_time).

I don't get what you guys are saying. How can you avoid the spin lock
around the two time register reads? How about a patch or some pseudo
code?

tmreg_lock now becomes seqlock_t instead of spinlock_t.

/* users can keep re-trying - dont really care */
igb_gettime_locking (...) {
     unsigned int seq;
     u64 ns;
     do {
     seq = read_seqbegin( &pigb->tmreg_seq_lock);
     ns = timecounter_read(&pigb->tc);
     } while (read_seqretry(&pigb->tmreg_seq_lock, seq));
     // process ns
}


copyright 2012 - Chetan Loke [off-list ref]

// trip cnt will ensure/enforce - evil adjtime user-space code can
still not block us.
// called from igb_tx[rx]_hwtstamp
driver_rx_tx_path_locking( ... ) {
     unsigned int seq, trip_cnt = 0;
     u64 ns;
     do {
     seq = read_seqbegin( &pigb->tmreg_seq_lock);
     trip_cnt++;
     ns = timecounter_read(&pigb->tc);
     } while (read_seqretry(&pigb->tmreg_seq_lock, seq) && trip_cnt < 2));
     // process ns
}


igb_adjtime () {
    /* just let first user of adjtime or settime succeed? */
    if (write_try_seqlock(&pigb->tmreg_seq_lock)) {
         // update NIC counter - here ...
         write_sequnlock(&pigb->tmreg_seq_lock);
    }
}

igb_settime () {
    // let all of them do their thing ... or you can use trylock here
too if you like...
    write_seqlock(&pigb->tmreg_seq_lock);
         // update NIC counter - here ...
    write_sequnlock(&pigb->tmreg_seq_lock);
 }

write_seq_xxx will spinlock as usual but read_seq reads the atomically
incremented counter. Also notice the use of trip_cnt to further ensure
the driver never keeps re-trying.

The regular __irqsave flavors are also available for use for the
write_seq calls.

Thanks,
Richard

Chetan