Re: [PATCH net-next] bnxt_en: replace PTP spinlock with seqlock

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 15/10/2024 07:20, Michael Chan wrote:
> On Mon, Oct 14, 2024 at 4:29 PM Vadim Fedorenko <vadfed@meta.com> wrote:
>>
>> We can see high contention on ptp_lock while doing RX timestamping
>> on high packet rates over several queues. Spinlock is not effecient
>> to protect timecounter for RX timestamps when reads are the most
>> usual operations and writes are only occasional. It's better to use
>> seqlock in such cases.
>>
>> Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
> 
>> -/* Caller holds ptp_lock */
>>   static int bnxt_refclk_read(struct bnxt *bp, struct ptp_system_timestamp *sts,
>>                              u64 *ns)
>>   {
>>          struct bnxt_ptp_cfg *ptp = bp->ptp_cfg;
>>          u32 high_before, high_now, low;
>>
>> +       /* Make sure the RESET bit is set */
>> +       smp_mb__before_atomic();
> 
> This may not be sufficient.  MMIO read of any register (clock register
> in this case) can hang the chip if it is undergoing reset.
> 
>>          if (test_bit(BNXT_STATE_IN_FW_RESET, &bp->state))
>>                  return -EIO;
> 
> We could have missed the flag and got here while the chip is about to be reset.
> 
> I will review the patch in more detail tomorrow.  Thanks.


Ok, so we have to serialize bnxt_refclk_read() and FW RESETS, but don't
block readers of ptp, especially on RX hot path. So it looks like
read_seqcount_excl_bh() can help us with it.