02/11/2018 08:15, Gavin Hu (Arm Technology China): > > > -----Original Message----- > > From: Honnappa Nagarahalli > > Sent: Friday, November 2, 2018 12:31 PM > > To: Gavin Hu (Arm Technology China) <gavin...@arm.com>; Stephen > > Hemminger <step...@networkplumber.org> > > Cc: dev@dpdk.org; tho...@monjalon.net; olivier.m...@6wind.com; > > chao...@linux.vnet.ibm.com; bruce.richard...@intel.com; > > konstantin.anan...@intel.com; jerin.ja...@caviumnetworks.com; > > sta...@dpdk.org; nd <n...@arm.com> > > Subject: RE: [PATCH v4 2/2] ring: move the atomic load of head above the > > loop > > > > <Fixing this to make the reply inline, making email plain text> > > > > > > On Thu, 1 Nov 2018 17:53:51 +0800 > > Gavin Hu <mailto:gavin...@arm.com> wrote: > > > > > +* **Updated the ring library with C11 memory model.** > > > + > > > + Updated the ring library with C11 memory model including the following > > changes: > > > + > > > + * Synchronize the load and store of the tail > > > + * Move the atomic load of head above the loop > > > + > > > > Does this really need to be in the release notes? Is it a user visible > > change or > > just an internal/optimization and fix. > > > > [Gavin] There is no api changes, but this is a significant change as ring is > > fundamental and widely used, it decreases latency by 25% in our tests, it > > may > > do even better for cases with more contending producers/consumers or > > deeper depth of rings. > > > > [Honnappa] I agree with Stephen. Release notes should be written from > > DPDK user perspective. In the rte_ring case, the user has the option of > > choosing between c11 and non-c11 algorithms. Performance would be one > > of the criteria to choose between these 2 algorithms. IMO, it probably makes > > sense to indicate that the performance of c11 based algorithm has been > > improved. However, I do not know what DPDK has followed historically > > regarding performance optimizations. I would prefer to follow whatever has > > been followed so far. > > I do not think that we need to document the details of the internal changes > > since it does not help the user make a decision. > > I read through the online guidelines for release notes, besides API and new > features, resolved issues which are significant and not newly introduced in > this release cycle, should also be included. > In this case, the resolved issue existed for long time, across multiple > release cycles and ring is a core lib, so it should be a candidate for > release notes. > > https://doc.dpdk.org/guides-18.08/contributing/patches.html > section 5.5 says: > Important changes will require an addition to the release notes in > doc/guides/rel_notes/. > See the Release Notes section of the Documentation Guidelines for details. > https://doc.dpdk.org/guides-18.08/contributing/documentation.html#doc-guidelines > "Developers should include updates to the Release Notes with patch sets that > relate to any of the following sections: > New Features > Resolved Issues (see below) > Known Issues > API Changes > ABI Changes > Shared Library Versions > Resolved Issues should only include issues from previous releases that have > been resolved in the current release. Issues that are introduced and then > fixed within a release cycle do not have to be included here." > > Suggested order in release notes items: > * Core libs (EAL, mempool, ring, mbuf, buses) > * Device abstraction libs and PMDs
I agree with Honnappa. You don't need to give details, but can explain that performance of C11 version is improved.