linux-kernel - Re: E1000: e1000_update

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <47CF0F1F.1080402@nortel.com>
Date:	Wed, 05 Mar 2008 15:22:39 -0600
From:	"Chris Friesen" <cfriesen@...tel.com>
To:	Todd Tomaino <ttomaino@...ulet.com>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: E1000:   e1000_update_stats()

Todd Tomaino wrote:

> Reading the comments in the code it appeared that updating
> adapter->net_stats and adapter->stat required interrupts to be turned
> off to prevent the e1000_adjust_tbi_stats() function from modifying
> these structures from interrupt context.  In order to reduce the
> amount of time spent with interrupts disabled I separated the
> register reads from updating adapter->stat counters.  The register
> reads take up the bulk of the 57 usec and shouldn't require
> protection (I could be wrong).  The new operation looks like this and
> reduces the amount of time spent with interrupts turned off by 1/2:
> 
> 1. read registers 2. acquire spinlock 3. update adapter->stat
> counters 4. update OS adapter->net_stats statistics 5. update phy
> stats. 6. release spinlock.

One potential problem with this is that the stats can be further out of 
date.

Consider this case:

cpu0 reads some of the registers, then gets preempted/interrupted
cpu1 reads all the registers, updates the stats, then dumps the data


At this point cpu1 dumped information that doesn't include the registers 
that cpu0 was in the middle of updating.  Eventually cpu0 will run again 
and the stats will be accurate, but there is a window where the stats 
may be self-inconsistent and the various counts may not add up.

Chris

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/