Re: [PATCH] printk: ringbuffer: Improve prb_next_seq() performance

[John Ogness <john.ogness@linutronix.de>]

On 2021-10-26, John Ogness <john.ogness@linutronix.de> wrote:
> If prb_last_finalized_seq() was successful (which can also be 0 on
> success) then there is no point reading that record. We already know
> that it must be higher.
>
> I suggest implementing everything within prb_next_seq() instead of
> adding the helper function prb_last_finalized_seq(). IMHO this cleanly
> handles the case of a failed read for the last finalized id and avoids
> an unnecessary descriptor read.
>
> u64 prb_next_seq(struct printk_ringbuffer *rb)
> {
> 	struct prb_desc_ring *desc_ring = &rb->desc_ring;
> 	enum desc_state d_state;
> 	unsigned long id;
> 	u64 seq;
>
> 	/* Check if the cached @id still points to a valid @seq. */
> 	id = atomic_long_read(&desc_ring->last_finalized_id);
> 	d_state = desc_read(desc_ring, id, NULL, &seq, NULL);
>
> 	if (d_state == desc_finalized || d_state == desc_reusable) {
> 		/* Begin searching after the last finalized record. */
> 		seq++;

I just realized that this doesn't work for the case when the ringbuffer
is empty (due to hack#2 of the bootstrapping). That would need to be
handled here. Something like:

 		/*
 		 * Begin searching after the last finalized record.
 		 * (On 0, the search must begin at 0 because of hack#2
 		 * of the bootstrapping phase it is not known if a
 		 * record at index 0 exists.)
 		 */
 		if (seq != 0)
 			seq++;

> 	} else {
> 		/*
> 		 * The information about the last finalized sequence number
> 		 * has gone. It should happen only when there is a flood of
> 		 * new messages and the ringbuffer is rapidly recycled.
> 		 * Give up and start from the beginning.
> 		 */
> 		seq = 0;
> 	}
>
> 	/*
> 	 * The information about the last finalized @seq might be inaccurate.
> 	 * Search forward to find the current one.
> 	 */
> 	while (_prb_read_valid(rb, &seq, NULL, NULL))
> 		seq++;
>
> 	return seq;
> }

John Ogness

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek