Re: [PATCH net-next v26 07/13] rtase: Implement a function to receive packets

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 7 Aug 2024 11:37:17 +0800 Justin Lai wrote:
> +static int rx_handler(struct rtase_ring *ring, int budget)
> +{
> +	union rtase_rx_desc *desc_base = ring->desc;
> +	u32 pkt_size, cur_rx, delta, entry, status;
> +	struct rtase_private *tp = ring->ivec->tp;
> +	struct net_device *dev = tp->dev;
> +	union rtase_rx_desc *desc;
> +	struct sk_buff *skb;
> +	int workdone = 0;
> +
> +	cur_rx = ring->cur_idx;
> +	entry = cur_rx % RTASE_NUM_DESC;
> +	desc = &desc_base[entry];
> +
> +	do {
> +		/* make sure discriptor has been updated */
> +		dma_rmb();

I think I already asked, but this still makes no sense to me.
dma barriers are between accesses to descriptors.
dma_rmb() ensures ordering of two reads.
Because modern CPUs can perform reads out of order.
What two reads is this barrier ordering?

Please read the doc on memory barriers relating to dma.
I think this is in the wrong place.

r8169 gets it right.

> +		status = le32_to_cpu(desc->desc_status.opts1);
> +
> +		if (status & RTASE_DESC_OWN)
> +			break;
> +