RE: [PATCH v2 net] net: solve a NAPI race

From: David Laight <David.Laight@ACULAB.COM>
To: 'Alexander Duyck' <alexander.duyck@gmail.com>,
	Eric Dumazet <eric.dumazet@gmail.com>
Cc: David Miller <davem@davemloft.net>,
	netdev <netdev@vger.kernel.org>,
	Tariq Toukan <tariqt@mellanox.com>,
	Saeed Mahameed <saeedm@mellanox.com>
Subject: RE: [PATCH v2 net] net: solve a NAPI race
Date: Wed, 1 Mar 2017 10:41:37 +0000	[thread overview]
Message-ID: <063D6719AE5E284EB5DD2968C1650D6DCFE727EB@AcuExch.aculab.com> (raw)
In-Reply-To: <CAKgT0UeSpxzgePoidJumQFvM5a2mOC5+rL8gCHqvX-HBvrn74A@mail.gmail.com>

From: Alexander Duyck
> Sent: 28 February 2017 17:20
...
> You might want to consider just using a combination AND, divide,
> multiply, and OR to avoid having to have any conditional branches
> being added due to this code path.  Basically the logic would look
> like:
>     new = val | NAPIF_STATE_SCHED;
>     new |= (val & NAPIF_STATE_SCHED) / NAPIF_STATE_SCHED * NAPIF_STATE_MISSED;
> 
> In assembler that all ends up getting translated out to AND, SHL, OR.
> You avoid the branching, or MOV/OR/TEST/CMOV type code you would end
> up with otherwise.

It is a shame gcc doesn't contain that optimisation.
It also doesn't even make a good job of (a & b)/b * c since it
always does a shr and a sal (gcc 4.7.3 and 5.4).

Worthy of a #define or static inline.
Something like:
#define BIT_IF(v, a, b) ((b & (b-1) ? (v & a)/a * b : a > b ? (v & a) / (a/b) : (v & a) * (b/a))

	David