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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 27 Mar 2012 20:05:01 +0200
From:	Richard Cochran <richardcochran@...il.com>
To:	"Keller, Jacob E" <jacob.e.keller@...el.com>
Cc:	chetan loke <loke.chetan@...il.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"Ronciak, John" <john.ronciak@...el.com>,
	"john.stultz@...aro.org" <john.stultz@...aro.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>
Subject: 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



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html