On 2019-03-18 11:21, Stanislaw Gruszka wrote:
> On Sat, Mar 16, 2019 at 09:42:40PM +0100, Felix Fietkau wrote:
>> Avoids expensive 64-bit atomic access in the data path
> <snip>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/mac.c
>> b/drivers/net/wireless/mediatek/mt76/mt7603/mac.c
>> index d6e260ca1423..fb1961ac9dc6 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt7603/mac.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt7603/mac.c
>> @@ -924,7 +924,11 @@ mt7603_mac_write_txwi(struct mt7603_dev *dev, __le32
>> *txwi,
>> txwi[3] = cpu_to_le32(val);
>>
>> if (key) {
>> - u64 pn = atomic64_inc_return(&key->tx_pn);
>> + u64 pn;
>> +
>> + spin_lock(&dev->mt76.lock);
>> + pn = ++wcid->tx_pn;
>> + spin_unlock(&dev->mt76.lock);
>
> It's interesting that atomic op is more expensive that taking spinlock
> and do the operation under spinlock protection. This should not be
> the case, atomic ops are provided exactly to avoid locking and should
> be faster than spin_lock. Perhaps on architecture where this happen
> (presumably MIPS 32bit) either atomic ops assembly code should be fixed
> or arch should switch generic lib/atomic64.c .
The fact that lib/atomic64.c is being used is the problem here.
The generic implementation takes a spin_lock_irqsave, which is more
expensive than the bh lock.
- Felix
