On 2019-06-30, Andrea Parri wrote:
>> The significant events for 2 contexts that are accessing the same
>> addresses of a descriptor are:
>>
>> P0(struct desc *d0)
>> {
>> // adding a new descriptor d0
>>
>> WRITE_ONCE(d0->next, EOL); // C
>> WRITE_ONCE(d0->
On Mon, Jul 01, 2019 at 12:39:35PM +0200, John Ogness wrote:
> On 2019-06-28, Peter Zijlstra wrote:
> > (Note that the qspinlock has a queue not unlike this, but that again
> > doesn't have to bother with NMIs)
>
> Thank you for pointing this out! I will look to qspinlock for some
> naming guidel
On Mon, Jul 01, 2019 at 12:39:35PM +0200, John Ogness wrote:
> Thanks. I overlooked that subtle detail. Can I assume NMIs do not exist
> on architectures that need to implement locking for cmpxchg()? Or did I
> just hit a major obstacle?
I think that is a fair assumption, I'm not aware of anybody
On 2019-06-28, Peter Zijlstra wrote:
>> I have implemented and tested these changes. I also added setting the
>> list terminator to this function, since all callers would have to do
>> it anyway. Also, I spent a lot of time trying to put in comments that
>> I think are _understandable_ and _accept
On Sun, Jun 30, 2019 at 04:03:34AM +0200, John Ogness wrote:
> On 2019-06-29, Andrea Parri wrote:
> >> /**
> >> * add_descr_list() - Add a descriptor to the descriptor list.
> >> *
> >> * @e: An entry that has already reserved data.
> >> *
> >> * The provided entry contains a pointer to a des
On 2019-06-29, Andrea Parri wrote:
>> /**
>> * add_descr_list() - Add a descriptor to the descriptor list.
>> *
>> * @e: An entry that has already reserved data.
>> *
>> * The provided entry contains a pointer to a descriptor that has already
>> * been reserved for this entry. However, the r
> /**
> * add_descr_list() - Add a descriptor to the descriptor list.
> *
> * @e: An entry that has already reserved data.
> *
> * The provided entry contains a pointer to a descriptor that has already
> * been reserved for this entry. However, the reserved descriptor is not
> * yet on the l
On Fri, Jun 28, 2019 at 05:44:35PM +0200, Peter Zijlstra wrote:
> On Fri, Jun 28, 2019 at 11:50:33AM +0200, John Ogness wrote:
> > cmpxchg_release() vs WRITE_ONCE() is not safe?! Can you point me to
> > documentation about this?
>
> Documentation/atomic_t.txt has this, see the SEMANTICS section o
On Fri, Jun 28, 2019 at 11:50:33AM +0200, John Ogness wrote:
> On 2019-06-27, Peter Zijlstra wrote:
> +static void add_descr_list(struct prb_reserved_entry *e)
> +{
> +struct printk_ringbuffer *rb = e->rb;
> +struct prb_list *l = &rb->descr_list;
> +
On 2019-06-27, Peter Zijlstra wrote:
>> This is a long response, but we are getting into some fine details
>> about the memory barriers (as well as battling my communication skill
>> level).
>
> So I'm going to reply piecewise to this... so not such long emails,
> but more of them.
I am not sure
On Thu, Jun 27, 2019 at 12:40:34AM +0200, Peter Zijlstra wrote:
> You have a single linked list going from the tail to the head, while
> adding to the head and removing from the tail. And that sounds like a
> FIFO queue:
>
> struct lqueue_head {
> struct lqueue_node *head, *tai
On Wed 2019-06-26 23:43:56, John Ogness wrote:
> Here is where I have massive problems communicating. I don't understand
> why you say the barrier is _between_ newest and next. I would say the
> barrier is _on_ newest to _synchronize_ with next (or something). I am
> struggling with terminology. (T
On Fri, Jun 21, 2019 at 12:23:19AM +0200, John Ogness wrote:
> On 2019-06-18, Peter Zijlstra wrote:
> >> +
> >> + if (unlikely(newest_id == EOL)) {
> >> + /* no previous newest means we *are* the list, set oldest */
> >> +
> >> + /*
> >> + * MB UNPAIRED
> >
> > That's
On Fri, Jun 21, 2019 at 12:23:19AM +0200, John Ogness wrote:
> Hi Peter,
>
> This is a long response, but we are getting into some fine details about
> the memory barriers (as well as battling my communication skill level).
So I'm going to reply piecewise to this... so not such long emails, but
On 2019-06-26, Peter Zijlstra wrote:
>> Here are the writer-relevant memory barriers and their associated
>> variables:
>>
>> MB1: data_list.oldest
>> MB2: data_list.newest
>> MB3: data_block.id
>> MB4: descr.data_next
>> MB5: descr_list.newest
>> MB6: descr.next
>
> I think this is the fundament
On Mon, Jun 24, 2019 at 10:33:15AM +0200, John Ogness wrote:
> Here are the writer-relevant memory barriers and their associated
> variables:
>
> MB1: data_list.oldest
> MB2: data_list.newest
> MB3: data_block.id
> MB4: descr.data_next
> MB5: descr_list.newest
> MB6: descr.next
I think this is th
On 2019-06-26, Petr Mladek 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
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 descrip
On (06/26/19 09:47), Petr Mladek wrote:
[..]
> If the data might change under the hood then we have bigger
> problems. For example, there might be a race when the trailing
> "\0" has not been written yet.
Current printk would not handle such cases. I'm only talking about
transition from one consis
On Wed 2019-06-26 09:16:11, John Ogness wrote:
> On 2019-06-26, Sergey Senozhatsky wrote:
> > [..]
> >> > CPU0 CPU1
> >> > printk(...)
> >> > sz = vscprintf(NULL, "Comm %s\n", current->comm);
> >> >
On (06/26/19 09:16), John Ogness wrote:
> On 2019-06-26, Sergey Senozhatsky wrote:
> > [..]
> >> In my v1 rfc series, I avoided this issue by having a separate dedicated
> >> ringbuffer (rb_sprintf) that was used to allocate a temporary max-size
> >> (2KB) buffer for sprinting to. Then _that_ was
On 2019-06-26, Sergey Senozhatsky wrote:
> [..]
>> > CPU0 CPU1
>> > printk(...)
>> > sz = vscprintf(NULL, "Comm %s\n", current->comm);
>> >
>> > ia64_mca_modify_comm()
>> >
On (06/25/19 14:03), John Ogness wrote:
[..]
> > CPU0CPU1
> > printk(...)
> > sz = vscprintf(NULL, "Comm %s\n", current->comm);
> >
> > ia64_mca_modify_comm()
> >
On 2019-06-24, Petr Mladek wrote:
>>> 1. Linked list of descriptors
>>> -
>>>
>>> The list of descriptors makes the code more complicated
>>> and I do not see much gain. It is possible that I just missed
>>> something.
>>>
>>> If I get it correctly then the list could o
On 2019-06-25, Sergey Senozhatsky wrote:
In vprintk_emit(), are we going to always reserve 1024-byte
records, since we don't know the size in advance, e.g.
printk("%pS %s\n", regs->ip, current->name)
prb_reserve(&e, &rb, );
or are we going to
On (06/25/19 11:06), Petr Mladek wrote:
> On Tue 2019-06-25 10:44:19, John Ogness wrote:
> > On 2019-06-25, Sergey Senozhatsky wrote:
> > > In vprintk_emit(), are we going to always reserve 1024-byte
> > > records, since we don't know the size in advance, e.g.
> > >
> > > printk("%pS %s\n", reg
On 2019-06-25, Sergey Senozhatsky wrote:
> [..]
>> +static void add_descr_list(struct prb_reserved_entry *e)
>> +{
>> +struct printk_ringbuffer *rb = e->rb;
>> +struct prb_list *l = &rb->descr_list;
>> +struct prb_descr *d = e->descr;
>> +struct prb_descr *newest_d;
>> +unsigne
On (06/25/19 10:44), John Ogness wrote:
> > In vprintk_emit(), are we going to always reserve 1024-byte
> > records, since we don't know the size in advance, e.g.
> >
> > printk("%pS %s\n", regs->ip, current->name)
> > prb_reserve(&e, &rb, );
> >
> > or are we going to run vsc
On Tue 2019-06-25 10:44:19, John Ogness wrote:
> On 2019-06-25, Sergey Senozhatsky wrote:
> > In vprintk_emit(), are we going to always reserve 1024-byte
> > records, since we don't know the size in advance, e.g.
> >
> > printk("%pS %s\n", regs->ip, current->name)
> > prb_reserve(
On (06/07/19 18:29), John Ogness wrote:
[..]
> +static void add_descr_list(struct prb_reserved_entry *e)
> +{
> + struct printk_ringbuffer *rb = e->rb;
> + struct prb_list *l = &rb->descr_list;
> + struct prb_descr *d = e->descr;
> + struct prb_descr *newest_d;
> + unsigned long
On 2019-06-25, Sergey Senozhatsky wrote:
+ struct prb_reserved_entry e;
+ char *s;
+
+ s = prb_reserve(&e, &rb, 32);
+ if (s) {
+ sprintf(s, "Hello, world!");
+ prb_commit(&e);
+ }
>>>
>>> A nit: snprintf().
>>>
>>> sprintf() is tricky
On (06/25/19 15:45), Sergey Senozhatsky wrote:
> On (06/19/19 00:12), John Ogness wrote:
> > On 2019-06-18, Sergey Senozhatsky wrote:
> > >> +struct prb_reserved_entry e;
> > >> +char *s;
> > >> +
> > >> +s = prb_reserve(&e, &rb, 32);
> > >> +if (s) {
> > >> +
On (06/19/19 00:12), John Ogness wrote:
> On 2019-06-18, Sergey Senozhatsky wrote:
> >> + struct prb_reserved_entry e;
> >> + char *s;
> >> +
> >> + s = prb_reserve(&e, &rb, 32);
> >> + if (s) {
> >> + sprintf(s, "Hello, world!");
> >> + prb_commit(&e);
> >> + }
> >
> > A ni
On Mon 2019-06-24 10:33:15, John Ogness wrote:
> > 1. Linked list of descriptors
> > -
> >
> > The list of descriptors makes the code more complicated
> > and I do not see much gain. It is possible that I just missed
> > something.
> >
> > If I get it correctly then the
Hi,
I would like to point out an important typo related to memory
barriers...
On 2019-06-07, John Ogness wrote:
> diff --git a/lib/printk_ringbuffer.c b/lib/printk_ringbuffer.c
> new file mode 100644
> index ..d0b2b6a549b0
> --- /dev/null
> +++ b/lib/printk_ringbuffer.c
[...]
> +stat
Hi Petr,
On 2019-06-21, Petr Mladek wrote:
> I am still scratching head around this. Anyway, I wanted to
> write something. I am sorry that the answer is really long.
> I do not know how to write it more effectively.
No need to apologize (to me) for long answers.
> 1. Linked list of descriptors
Hi John,
I am still scratching head around this. Anyway, I wanted to
write something. I am sorry that the answer is really long.
I do not know how to write it more effectively.
First, the documentation helped a lot. Also I found several
ideas that were important to make it working a lockless way.
> > FWIW (and as anticipated time ago in a private email), when I see code
> > like this I tend to look elsewhere... ;-/
>
> Do you really mean "code" or are you just referring to "code comments"?
> If you really mean code, then I'd appreciate some feedback about what
> should change.
I really j
On 2019-06-19, Andrea Parri wrote:
>> I would appreciate it if you could point out a source file that
>> documents its memory barriers the way you would like to see these memory
>> barriers documented.
>
> IMO, you could find some inspiration by looking at the memory barriers
> comments from:
>
>
Hi Peter,
This is a long response, but we are getting into some fine details about
the memory barriers (as well as battling my communication skill level).
On 2019-06-18, Peter Zijlstra wrote:
>> +#define DATAARRAY_SIZE(rb) (1 << rb->data_array_size_bits)
>> +#define DATAARRAY_SIZE_BITMASK(rb) (D
On Wed, Jun 19, 2019 at 12:30:26AM +0200, John Ogness wrote:
> On 2019-06-18, Peter Zijlstra wrote:
> >> +/**
> >> + * DOC: memory barriers
> >
> > What's up with that 'DOC' crap?
>
> The separate documentation in
> Documentation/core-api/printk-ringbuffer.rst references this so it
> automaticall
> I would appreciate it if you could point out a source file that
> documents its memory barriers the way you would like to see these memory
> barriers documented.
IMO, you could find some inspiration by looking at the memory barriers
comments from:
kernel/sched/core.c:try_to_wake_up()
includ
On 2019-06-18, Peter Zijlstra wrote:
>> +/**
>> + * DOC: memory barriers
>
> What's up with that 'DOC' crap?
The separate documentation in
Documentation/core-api/printk-ringbuffer.rst references this so it
automatically shows up in the kernel docs. An external reference
requires the DOC keyword.
On 2019-06-18, Peter Zijlstra wrote:
>> +/**
>> + * struct prb_descr - A descriptor representing an entry in the ringbuffer.
>> + * @seq: The sequence number of the entry.
>> + * @id: The descriptor id.
>> + * The location of the descriptor within the descriptor array can be
>> + * dete
On 2019-06-18, Sergey Senozhatsky wrote:
>> +struct prb_reserved_entry e;
>> +char *s;
>> +
>> +s = prb_reserve(&e, &rb, 32);
>> +if (s) {
>> +sprintf(s, "Hello, world!");
>> +prb_commit(&e);
>> +}
>
> A nit: snprintf().
>
> sprintf() is tricky, it may w
On Fri, Jun 07, 2019 at 06:29:48PM +0206, John Ogness wrote:
> +#define DATAARRAY_SIZE(rb) (1 << rb->data_array_size_bits)
> +#define DATAARRAY_SIZE_BITMASK(rb) (DATAARRAY_SIZE(rb) - 1)
*phew* no comments on those..
I think the kernel typically uses _MASK instead of _BITMASK for this
though.
> +
On Fri, Jun 07, 2019 at 06:29:48PM +0206, John Ogness wrote:
> +/**
> + * DOC: memory barriers
What's up with that 'DOC' crap?
> + *
> + * Writers
> + * ---
> + * The main issue with writers is expiring/invalidating old data blocks in
> + * order to create new data blocks. This is performed i
On Fri, Jun 07, 2019 at 06:29:48PM +0206, John Ogness wrote:
> +/**
> + * struct prb_descr - A descriptor representing an entry in the ringbuffer.
> + * @seq: The sequence number of the entry.
> + * @id: The descriptor id.
> + * The location of the descriptor within the descriptor array can be
Hello John,
On (06/07/19 18:29), John Ogness wrote:
[..]
> + struct prb_reserved_entry e;
> + char *s;
> +
> + s = prb_reserve(&e, &rb, 32);
> + if (s) {
> + sprintf(s, "Hello, world!");
> + prb_commit(&e);
> + }
A nit: snprintf().
sprintf() is tricky,
See documentation for details.
Signed-off-by: John Ogness
---
Documentation/core-api/index.rst | 1 +
Documentation/core-api/printk-ringbuffer.rst | 104 +++
include/linux/printk_ringbuffer.h| 238 +++
lib/printk_ringbuffer.c | 924 +
50 matches
Mail list logo
