Message-ID: <20190626082935.ocbqqaol5jzcuxwl@pathway.suse.cz>
Date:   Wed, 26 Jun 2019 10:29:35 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     John Ogness <john.ogness@...utronix.de>
Cc:     linux-kernel@...r.kernel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andrea Parri <andrea.parri@...rulasolutions.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [RFC PATCH v2 1/2] printk-rb: add a new printk ringbuffer
 implementation

On Tue 2019-06-25 15:29:35, John Ogness wrote:
> To address your question: For the linked list implementation, if you are
> looking at it from the linked list perspective, the number of
> descriptors on the list is constantly fluctuating (increasing and
> decreasing) and the ordering of the descriptors is constantly
> changing. They are ordered according to the writer commit order (not the
> writer reserve order) and the only descriptors on the list are the ones
> that are not within a reserve/commit window.

This and few other comments below are really valuable explanation.
I misunderstood how the list worked. I have to revisit it and
rethink my view on the patchset.

> >>> If the above is true then we could achieve similar result
> >>> when using the array as a circular buffer. It would be
> >>> the same like when all members are linked from the beginning.
> >> 
> >> So you are suggesting using a multi-reader multi-writer lockless
> >> ringbuffer to implement a multi-reader multi-writer lockless
> >> ringbuffer. ;-)
> >> 
> >> The descriptor ringbuffer has fixed-size items, which simplifies the
> >> task. But I expect you will run into a chicken-egg scenario.
> >
> > AFAIK, the main obstacle with the fully lockless solution was
> > that the entries did not have a fixed size.
> 
> No. The variable size of the records was the reason I used
> descriptors. That has nothing to do with how I chose to connect those
> descriptors.

I think that we are talking about the same. If I remember correctly,
the main problem is that cmpxchg() is not reliable when the same
address might be used by the metadata and data.

For example, the code never know if it compared previous seq number
of it another CPU/NMI wrote there data (string) in the meantime.

> I think it is not as simple as you think it is and I expect you will end
> up with a solution that is more complex (although I could be
> wrong). IMHO the linked list solution is quite elegant.

It is quite likely.

> There is an obvious push to get the kernel docs unified under RST, even
> if it is not how I usually do things either. However, now that I've done
> the work, looking back it seems to be a good idea in order to automate
> documentation.

I personally like /** */ description of public API functions. Also
html/pdf version looks nice even though I do not use them.

The thing is that both /** */ and .rst formats can be well readable
even in the source code. I guess that most existing developers read
only the source code.

Well, this discussion probably belongs to another thread. My wish
was just to make the commit message more verbose, please.

> I explicitly wanted to get away from any preconceptions. By specifying
> we have data linked from oldest to newest, I find it feels more natural,
> regardless if I am a writer writing new records to the newest or a
> reader reading all records from oldest to newest.

As I said, I do not have strong opinion. I could live with oldest, newest.

Best Regards,
Petr

Powered by blists - more mailing lists