Re: 答复: 答复: [PATCH v8 12/17] net/r8169: implement Tx path

[Thomas Monjalon <thomas@monjalon.net>]

Yes atomic load is perfect.
rte_rmb is worst :)


19/11/2024 09:52, 王颢:
> Dear Thomas,
> 
> How about the following modification? Or could I just change rte_smp_rmb to rte_rmb instead?
> 
> struct rtl_tx_desc {
> 	RTE_ATOMIC(u32) opts1;
> 	u32 opts2;
> 	u64 addr;
> 	u32 reserved0;
> 	u32 reserved1;
> 	u32 reserved2;
> 	u32 reserved3;
> };
> 
> static u32
> rtl_get_opts1(struct rtl_tx_desc *txd)
> {
>     u32 opts1 = rte_atomic_load_explicit (&txd->opts1, rte_memory_order_acquire);
> 
> 	return rte_le_to_cpu_32(opts1);
> }
> 
> 
> Best Wishes,
> Howard Wang
> 
> -----邮件原件-----
> 发件人: Thomas Monjalon <thomas@monjalon.net> 
> 发送时间: 2024年11月19日 15:16
> 收件人: pro_nic_dpdk@realtek.com; 王颢 <howard_wang@realsil.com.cn>
> 抄送: dev@dpdk.org; Ferruh Yigit <ferruh.yigit@amd.com>
> 主题: Re: 答复: [PATCH v8 12/17] net/r8169: implement Tx path
> 
> 
> External mail.
> 
> 
> 
> The change below is replacing 1 rte_smp_rmb with 2 calls, so no it is not what I am asking for.
> Please could you check how to not calling this function at all?
> 
> This series is already merged in the main branch, so any new change should be submitted as a new patch.
> 
> Thank you
> 
> 
> 19/11/2024 07:29, 王颢:
> > Dear Thomas,
> >
> > After our discussion, we concluded that we can make the following changes. What do you think? Additionally, I may not have been very clear in my last email. Should I submit the entire series of changes as PATCH v9, or should I respond with [PATCH v9 12/17] net/r8169: implement Tx path?
> >
> > static u32
> > rtl_get_opts1(struct rtl_tx_desc *txd) {
> > -     rte_smp_rmb();
> >
> >       return rte_le_to_cpu_32(txd->opts1); }
> >
> > static void
> > rtl_tx_clean(struct rtl_hw *hw, struct rtl_tx_queue *txq) {
> >               ...
> >               tx_left = (rte_atomic_load_explicit(&txq->tx_tail,
> >                                                   rte_memory_order_relaxed) % nb_tx_desc) - head;
> >       }
> >
> > +     rte_smp_rmb();
> >
> >       while (tx_left > 0) {
> >               txd = &txq->hw_ring[head];
> >
> >               if (!enable_tx_no_close && (rtl_get_opts1(txd) & DescOwn))
> >                       break;
> >               ...
> > }
> >
> >
> > int
> > rtl_tx_done_cleanup(void *tx_queue, uint32_t free_cnt) {
> >               ...
> >               tx_left = (rte_atomic_load_explicit(&txq->tx_tail,
> >                                                   rte_memory_order_relaxed) % nb_tx_desc) - head;
> >       }
> >
> > +     rte_smp_rmb();
> >
> >       while (tx_left > 0) {
> >               txd = &txq->hw_ring[head];
> >
> >               status = rtl_get_opts1(txd);
> >
> >               if (!enable_tx_no_close && (status & DescOwn))
> >                       break;
> >
> >               ...
> > }
> >
> > Best Regards,
> > Howard Wang
> >
> > -----邮件原件-----
> > 发件人: Thomas Monjalon <thomas@monjalon.net>
> > 发送时间: 2024年11月18日 22:59
> > 收件人: pro_nic_dpdk@realtek.com; 王颢 <howard_wang@realsil.com.cn>
> > 抄送: dev@dpdk.org; Ferruh Yigit <ferruh.yigit@amd.com>
> > 主题: Re: [PATCH v8 12/17] net/r8169: implement Tx path
> >
> >
> > External mail.
> >
> >
> >
> > Hello,
> >
> > 13/11/2024 10:28, Howard Wang:
> > > +static u32
> > > +rtl_get_opts1(struct rtl_tx_desc *txd) {
> > > +       rte_smp_rmb();
> > > +
> > > +       return rte_le_to_cpu_32(txd->opts1); }
> >
> > We should avoid using such heavy memory barrier.
> > Is it possible to use a lighter fence or atomic API?
> >
> >
> >
> 
> 
> 
> 
> 
>