On Wed, Sep 29, 2021 at 09:52:20AM +0000, Ananyev, Konstantin wrote:
>
>
> > -----Original Message-----
> > From: Xueming(Steven) Li <xuemi...@nvidia.com>
> > Sent: Wednesday, September 29, 2021 10:13 AM
<snip>
> > > + /* Locate real source fs according to mbuf->port. */
> > > + for (i = 0; i < nb_rx; ++i) {
> > > + rte_prefetch0(pkts_burst[i + 1]);
> > >
> > > you access pkt_burst[] beyond array boundaries,
> > > also you ask cpu to prefetch some unknown and possibly invalid
> > > address.
> >
> > Sorry I forgot this topic. It's too late to prefetch current packet, so
> > perfetch next is better. Prefetch an invalid address at end of a look
> > doesn't hurt, it's common in DPDK.
>
> First of all it is usually never 'OK' to access array beyond its bounds.
> Second prefetching invalid address *does* hurt performance badly on many CPUs
> (TLB misses, consumed memory bandwidth etc.).
> As a reference: https://lwn.net/Articles/444346/
> If some existing DPDK code really does that - then I believe it is an issue
> and has to be addressed.
> More important - it is really bad attitude to submit bogus code to DPDK
> community
> and pretend that it is 'OK'.
>
The main point we need to take from all this is that when
prefetching you need to measure perf impact of it.
In terms of the specific case of prefetching one past the end of the array,
I would take the view that this is harmless in almost all cases. Unlike any
prefetch of "NULL" as in the referenced mail, reading one past the end (or
other small number of elements past the end) is far less likely to cause a
TLB miss - and it's basically just reproducing behaviour we would expect
off a HW prefetcher (though those my explicitly never cross page
boundaries). However, if you feel it's just cleaner to put in an
additional condition to remove the prefetch for the end case, that's ok
also - again so long as it doesn't affect performance. [Since prefetch is a
hint, I'm not sure if compilers or CPUs may be legally allowed to skip the
branch and blindly prefetch in all cases?]
/Bruce
