RE: [PATCH v7] eal: add seqlock

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Mattias Rönnblom [mailto:hofors@lysator.liu.se]
> Sent: Sunday, 15 May 2022 19.54
> 
> On 2022-05-15 17:19, Morten Brørup wrote:
> >> From: Mattias Rönnblom [mailto:mattias.ronnblom@ericsson.com]
> >> Sent: Sunday, 15 May 2022 14.40
> >>
> >> Two questions remain:
> >>
> >> 1) Should the seqlock and the seqcount reside in different header
> >> files?
> >> 2) Is it it good enough to provided only a spinlock-protected
> seqlock?
> >>
> >> Question 1 I don't really have an opinion on. Both ways seems
> perfectly
> >> reasonable to me. I noted Morten wanted a split, and left to my own
> >> devices this is probably what I would do as well.
> >
> > Argument for separate header files: If we add e.g. a
> rte_seqticketlock_t later, it should be able to include the
> rte_seqcount_t header file without also getting the contextually
> irrelevant rte_seqlock_t type and functions.
> >
> > I don't feel strongly about this.
> >
> >>
> >> I think the answer to 2 is yes. We can provide other variants in the
> >> future, would the need arise.
> >
> > Agree.
> >
> >>
> >> <snip>
> >
> > Please move the header files from /lib/eal/include to
> /lib/eal/include/generic, where the other lock header files reside.
> >
> >
> 
> My guess would be that the lib/eal/include/generic directory is where
> the generic implementations of APIs for which there also exist
> machine-specific implementations go. The seqlock is not an example of
> such.

My guess would be the same. However, it is not documented what goes where in the DPDK tree structure, and the community seems to have a preference for following conventions rather than doing things right. So just turn off your brain and be a lemming for a second! ;-)

Or try your luck. You have my support for the patch regardless. :-)

<rant>
I think that most of the EAL has grown into a huge pile of monolithic spaghetti, with very little modularity. E.g. I suggested refactoring and fixing the mempool cache, but could not gather traction. Instead, an increasing amount of the mempool library's implementation details are being promoted from internal to public, and its internal code is being copy-pasted into other modules, so they can bypass both the mbuf and mempool libraries when allocating and freeing mbufs.
</rant>