Re: numlist API Re: [RFC PATCH v4 1/9] printk-rb: add a new printk ringbuffer implementation
From: John Ogness
Date: Wed Aug 28 2019 - 03:13:49 EST
On 2019-08-27, Petr Mladek <pmladek@xxxxxxxx> wrote:
> The API is complicated because of the callbacks. It depends on a logic
> that is implemented externally. It makes it abstract to some extent.
> My view is that the API would be much cleaner and easier to review
> when the ID handling is "hardcoded" (helper functions). It could be
> made abstract anytime later when there is another user.
> There should always be a reason why to make a code more complicated
> than necessary. It seems that the only reason is some theoretical
> future user and its theoretical requirements.
FWIW, I did _not_ create the numlist and dataring structures in order to
support some theoretical future user. PeterZ helped me realize that
RFCv2 was actually using multiple internal data structures. Each of
these internal data structures has their own set of memory barriers and
semantics. By explicitly refactoring them behind strong APIs, the memory
barriers could be clearly visible and the semantics clearly defined.
For me this was a great help in _simplifying_ the design. For me it also
greatly simplified debugging, testing, and verifying because I could
write tests for numlist and datalist that explicitly targeted those data
structures. Once I believed they were bullet-proof, I could move on to
higher-level tests of the printk_ringbuffer. And once I believed the
printk_ringbuffer was bullet-proof, I could move on to the higher-level
printk tests. When a problem was found, I could effectively isolate
which component failed their job.
I understand that we disagree about the abstractions being a
simplification. And I'm not sure how to proceed in this regard. (Maybe
once we get everything bullet-proof, we can put everything back together
into a monolith like RFCv2.) Either way, please understand that the
abstractions were done for the benefit of printk_ringbuffer, not for any
theoretical future user.