From: Martin Pieuchot <mpi@grenadille.net>
Subject: Re: dt: Continue read(2) from last read CPU ring
To: Christian Ludwig <cludwig@genua.de>
Cc: tech@openbsd.org
Date: Sat, 28 Sep 2024 14:48:22 +0200

On 15/09/24(Sun) 21:42, Christian Ludwig wrote:
> Hi,
> 
> on top of the diff to use one ringbuffer per CPU in dt(4) ([1]), we can
> easily reduce starvation of buffers for higher CPUs.
> 
> [1] https://marc.info/?l=openbsd-tech&m=172606806422278
> 
> When btrace continues reading events from busy ringbuffers, it always
> starts reading the first buffers. But it might never reach the later
> ones if the first ones fill up quickly.
> 
> Remeber the position and continue from the last ringbuffer that we read
> before.
> 
> So long,

I believe now would be the time to commit the whole per-CPU ringbuffer
change.  Could you integrate this change in the previous one?

> ---
>  sys/dev/dt/dt_dev.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/sys/dev/dt/dt_dev.c b/sys/dev/dt/dt_dev.c
> index 8adb165662e6..c0357239b98e 100644
> --- a/sys/dev/dt/dt_dev.c
> +++ b/sys/dev/dt/dt_dev.c
> @@ -111,6 +111,7 @@ struct dt_ringbuf {
>   *  Locks used to protect struct members in this file:
>   *	a	atomic
>   *	K	kernel lock
> + *	r	owned by read(2) proc
>   *	I	invariant after initialization
>   */
>  struct dt_softc {
> @@ -124,6 +125,7 @@ struct dt_softc {
>  	unsigned int		 ds_evtcnt;	/* [a] # of readable evts */
>  
>  	struct dt_ringbuf	 ds_ring[MAXCPUS]; /* [I] event rings */
> +	unsigned int		 ds_lastcpu;	/* [r] last CPU ring read(2). */
>  
>  	/* Counters */
>  	unsigned int		 ds_readevt;	/* [a] # of events read */
> @@ -220,6 +222,7 @@ dtopen(dev_t dev, int flags, int mode, struct proc *p)
>  	sc->ds_unit = unit;
>  	sc->ds_pid = p->p_p->ps_pid;
>  	TAILQ_INIT(&sc->ds_pcbs);
> +	sc->ds_lastcpu = 0;
>  	sc->ds_evtcnt = 0;
>  	sc->ds_readevt = 0;
>  	sc->ds_dropevt = 0;
> @@ -290,7 +293,7 @@ dtread(dev_t dev, struct uio *uio, int flags)
>  	KERNEL_ASSERT_LOCKED();
>  	for (i = 0; i < ncpusfound; i++) {
>  		count = 0;
> -		dr = &sc->ds_ring[i];
> +		dr = &sc->ds_ring[(sc->ds_lastcpu + i) % ncpusfound];
>  		error = dt_ring_copy(dr, uio, max, &count, &dropped);
>  		if (error && count == 0)
>  			break;
> @@ -300,6 +303,7 @@ dtread(dev_t dev, struct uio *uio, int flags)
>  		if (max == 0)
>  			break;
>  	}
> +	sc->ds_lastcpu += i % ncpusfound;
>  
>  	atomic_add_int(&sc->ds_readevt, read);
>  	atomic_add_int(&sc->ds_dropevt, dropped);
> -- 
> 2.34.1
>