Hi Jerin,
> -----Original Message----- > From: Jerin Jacob <jerinjac...@gmail.com> > Sent: Friday, January 3, 2020 3:35 PM > To: Gavin Hu <gavin...@arm.com> > Cc: dpdk-dev <dev@dpdk.org>; nd <n...@arm.com>; David Marchand > <david.march...@redhat.com>; tho...@monjalon.net; > rasl...@mellanox.com; maxime.coque...@redhat.com; tiwei....@intel.com; > hemant.agra...@nxp.com; jer...@marvell.com; Pavan Nikhilesh > <pbhagavat...@marvell.com>; Honnappa Nagarahalli > <honnappa.nagaraha...@arm.com>; Ruifeng Wang > <ruifeng.w...@arm.com>; Phil Yang <phil.y...@arm.com>; Joyce Kong > <joyce.k...@arm.com>; Steve Capper <steve.cap...@arm.com> > Subject: Re: [dpdk-dev] [PATCH v2 1/3] eal/arm64: relax the io barrier for > aarch64 > > On Fri, Jan 3, 2020 at 12:00 PM Gavin Hu <gavin...@arm.com> wrote: > > > > Hi Jerin, > > Hi Gavin, > > > > > > -----Original Message----- > > > From: Jerin Jacob <jerinjac...@gmail.com> > > > Sent: Thursday, January 2, 2020 5:52 PM > > > To: Gavin Hu <gavin...@arm.com> > > > Cc: dpdk-dev <dev@dpdk.org>; nd <n...@arm.com>; David Marchand > > > <david.march...@redhat.com>; tho...@monjalon.net; > > > rasl...@mellanox.com; maxime.coque...@redhat.com; > tiwei....@intel.com; > > > hemant.agra...@nxp.com; jer...@marvell.com; Pavan Nikhilesh > > > <pbhagavat...@marvell.com>; Honnappa Nagarahalli > > > <honnappa.nagaraha...@arm.com>; Ruifeng Wang > > > <ruifeng.w...@arm.com>; Phil Yang <phil.y...@arm.com>; Joyce Kong > > > <joyce.k...@arm.com>; Steve Capper <steve.cap...@arm.com> > > > Subject: Re: [dpdk-dev] [PATCH v2 1/3] eal/arm64: relax the io barrier for > > > aarch64 > > > > > > On Mon, Dec 23, 2019 at 3:46 PM Gavin Hu <gavin...@arm.com> wrote: > > > > > > > > Hi Jerin, > > > > > > > > > -----Original Message----- > > > > > From: Jerin Jacob <jerinjac...@gmail.com> > > > > > Sent: Monday, December 23, 2019 5:20 PM > > > > > To: Gavin Hu <gavin...@arm.com> > > > > > Cc: dpdk-dev <dev@dpdk.org>; nd <n...@arm.com>; David Marchand > > > > > <david.march...@redhat.com>; tho...@monjalon.net; > > > > > rasl...@mellanox.com; maxime.coque...@redhat.com; > > > > > tiwei....@intel.com; hemant.agra...@nxp.com; jer...@marvell.com; > > > > > Pavan Nikhilesh <pbhagavat...@marvell.com>; Honnappa Nagarahalli > > > > > <honnappa.nagaraha...@arm.com>; Ruifeng Wang > > > > > <ruifeng.w...@arm.com>; Phil Yang <phil.y...@arm.com>; Joyce > Kong > > > > > <joyce.k...@arm.com>; Steve Capper <steve.cap...@arm.com> > > > > > Subject: Re: [dpdk-dev] [PATCH v2 1/3] eal/arm64: relax the io barrier > for > > > > > aarch64 > > > > > > > > > > On Mon, Dec 23, 2019 at 2:44 PM Gavin Hu <gavin...@arm.com> > wrote: > > > > > > > > > > > > Hi Jerin, > > > > > > > > > > Hi Gavin, > > > > > > > > > > > > > > > > > I think we are on the same page with regard to the problem, and the > > > > > situations, thanks for illuminating the historical background of the > > > > > two > > > > > barriers. > > > > > > About the solution, I added inline comments. > > > > > > > It will be optimization only when if we are changing in the fast > > > > > > > path. > > > > > > > In the slow path, it does not matter. > > > > > > > I think, the First step should be to use rte_cio_* wherever it is > > > > > > > coherent memory used in _fast path_. I think, Almost every driver > > > > > > > fixed that. > > > > > > > > > > > > > > I am not against this patch(changing the slow path to use rte_cio* > > > > > > > from rte_io* and virtio changes associated with that). > > > > > > > If you are taking that patch, pay attention to all the drivers in > > > > > > > the > > > > > > > tree which is using rte_io* for mixed access in slowpath. > > > > > > I see 30+ drivers has calling rte_io* directly or indirectly through > > > > > rte_write/read*. > > > > > > It is hard for me to figure out all the mixed accesses in these > > > > > > drivers, > and > > > > > as you said, it makes no sense to change the _slow path_. > > > > > > > > > > > > How about we keep the old rte_io as is, and introduce 'fast path' > version > > > > > of rte_io for new code use? > > > > > > Then in future, we may merge the two? > > > > > > Another reason about this proposal is maybe there is rte_io calling > > > > > > in > > > the > > > > > fast path, but they are not mixed accesses and rte_cio is not > > > > > suitable. > > > > > > > > > > Could you share more details about the case where fastpath + rte_io > > > > > needed + rte_cio is not suitable? > > > > > > > > Here is an example for i40e, in the fast path, but only a pure io memory > > > access. > > > > > > > > https://code.dpdk.org/dpdk/v19.11/source/drivers/net/i40e/i40e_rxtx.c#L12 > > > 08 > > > > > > Yes. That's a performance issue. > > > > > > It could be changed to following for the fix that works on x86, arm64 > > > with existing infra. > > > > > > From: > > > I40E_PCI_REG_WRITE() > > > > > > to: > > > > > > rte_cio_wmb() > > > I40E_PCI_REG_WRITE_RELAXED() > > Yes, this is correct, I will submit a new patch for this. > > This is an example out of all the cases that I must fix before relaxing the > rte_io barriers. > > My plan is as follows, any comments are welcome! > > 1. replace rte_*mb and rte_io_*mb with rte_cio_*mb where applicable in > the fastpath, this is an optimization, as the barriers are relaxed. > > 2. replace all the rte_io_*mb with rte_cio_*mb where applicable in the > slowpath and control path > > 3. until *all* the occurrences in the step 1 and 2 are done, then this path > > can > be re-activated. > > > > Please advise if the above approach works from your viewpoint. > > I would prefer to have ONLY the step (1) and add a note for the same > in https://doc.dpdk.org/guides/prog_guide/perf_opt_guidelines.html > as documentation reference. Ok, good idea, I will submit a documentation patch for this. > > > > Maybe I will stop at step 1, step 2 and 3 are not necessary as they are not > > in > the fastpath? > > Yup. > > > > > > > > > > > > > > I wanted two variants of rte_io, because also x86 requires two as > indicated > > > here, one for no-WC and another for WC. > > > > http://inbox.dpdk.org/dev/20191204151916.12607-1- > > > xiaoyun...@intel.com/T/#ea8bb1b4a378ab09baedbf95b4542bcb92f4a396f > > > > > > > > > > > > > > > > > Any thoughts? > > > > > > > > > > > > > > > > > > > > > But as the case in i40e, we must pay attention to where rte_cio > was > > > > > > > missing but rescued by old rte_io(but not by new rte_io). > > > > > > > > > > > > > > > >
