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 newest_id;
> +
> + /* set as newest */
> + do {
> + /* MB5: synchronize add descr */
> + newest_id = smp_load_acquire(&l->newest);
> + newest_d = TO_DESCR(rb, newest_id);
> +
> + if (newest_id == EOL)
> + WRITE_ONCE(d->seq, 1);
> + else
> + WRITE_ONCE(d->seq, READ_ONCE(newest_d->seq) + 1);
> + /*
> + * MB5: synchronize add descr
> + *
> + * In particular: next written before cmpxchg
> + */
> + } while (cmpxchg_release(&l->newest, newest_id, e->id) != newest_id);
> +
> + if (unlikely(newest_id == EOL)) {
> + /* no previous newest means we *are* the list, set oldest */
> +
> + /*
> + * MB UNPAIRED
> + *
> + * In particular: Force cmpxchg _after_ cmpxchg on newest.
> + */
> + WARN_ON_ONCE(cmpxchg_release(&l->oldest, EOL, e->id) != EOL);
> + } else {
> + /* link to previous chain */
> +
> + /*
> + * MB6: synchronize link descr
> + *
> + * In particular: Force cmpxchg _after_ cmpxchg on newest.
> + */
> + WARN_ON_ONCE(cmpxchg_release(&newest_d->next,
> + EOL, e->id) != EOL);
> + }
> +}
[..]
> +char *prb_reserve(struct prb_reserved_entry *e, struct printk_ringbuffer *rb,
> + unsigned int size)
> +{
> + struct prb_datablock *b;
> + struct prb_descr *d;
> + char *buf;
> +
> + if (size == 0)
> + return NULL;
> +
> + size += sizeof(struct prb_datablock);
> + size = DATA_ALIGN_SIZE(size);
> + if (size > DATAARRAY_SIZE(rb))
> + return NULL;
> +
> + e->rb = rb;
> +
> + local_irq_save(e->irqflags);
> +
> + if (!assign_descr(e))
> + goto err_out;
> +
> + d = e->descr;
> + WRITE_ONCE(d->id, e->id);
> +
> + if (!data_reserve(e, size)) {
> + /* put invalid descriptor on list, can still be traversed */
> + WRITE_ONCE(d->next, EOL);
> + add_descr_list(e);
> + goto err_out;
> + }
I'm wondering if prb can always report about its problems. Including the
cases when things "go rather bad".
Suppose we have
printk()
prb_reserve()
!data_reserve()
add_descr_list()
WARN_ON_ONCE()
printk()
prb_reserve()
!assign_descr(e) << lost WARN_ON's "printk" or "printks"?
In general, assuming that there might be more error printk-s either
called directly directly from prb->printk on indirectly, from
prb->ABC->printk.
Also note,
Lost printk-s are not going to be accounted as 'lost' automatically.
It seems that for printk() there is no way to find out that it has
recursed from printk->prb_commit but hasn't succeeded in storing
recursive messages. I'd say that prb_reserve() err_out should probably
&rb->lost++.
-ss
