From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: dt: Count skipped clock cycles as dropped events
To: Christian Ludwig <cludwig@genua.de>
Cc: tech@openbsd.org, Martin Pieuchot <mpi@grenadille.net>
Date: Tue, 5 Nov 2024 15:45:46 +0100

On Tue, Nov 05, 2024 at 02:07:03PM +0100, Christian Ludwig wrote:
> If the clock interrupt advances for more than one tick, we obviously
> lost a number of clock ticks. That might happen in a virtual machine
> that does not get scheduled in time. There is no point in creating a
> cluster of events with basically the same timestamp and identical stack
> traces. Generate only one event instead and drop all skipped ticks.

Agreed, diff looks ok to me.
 
>  - Christian
> 
> ---
>  sys/dev/dt/dt_dev.c          | 12 ++++++++++--
>  sys/dev/dt/dt_prov_profile.c | 17 ++++++++++-------
>  sys/dev/dt/dtvar.h           |  1 +
>  3 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/sys/dev/dt/dt_dev.c b/sys/dev/dt/dt_dev.c
> index 75fbbba6a575..286bda80992d 100644
> --- a/sys/dev/dt/dt_dev.c
> +++ b/sys/dev/dt/dt_dev.c
> @@ -725,6 +725,15 @@ dt_pcb_purge(struct dt_pcb_list *plist)
>  	}
>  }
>  
> +void
> +dt_pcb_ring_drop(struct dt_pcb *dp, unsigned int drop)
> +{
> +	struct dt_cpubuf *dc = &dp->dp_sc->ds_cpu[cpu_number()];
> +
> +	dc->dc_dropevt += drop;
> +	membar_producer();
> +}
> +
>  /*
>   * Get a reference to the next free event state from the ring.
>   */
> @@ -747,8 +756,7 @@ dt_pcb_ring_get(struct dt_pcb *dp, int profiling)
>  	distance = prod - cons;
>  	if (distance == 1 || distance == (1 - DT_EVTRING_SIZE)) {
>  		/* read(2) isn't finished */
> -		dc->dc_dropevt++;
> -		membar_producer();
> +		dt_pcb_ring_drop(dp, 1);
>  
>  		dc->dc_inevt = 0;
>  		return NULL;
> diff --git a/sys/dev/dt/dt_prov_profile.c b/sys/dev/dt/dt_prov_profile.c
> index 62900152e147..5e4d9f1a2081 100644
> --- a/sys/dev/dt/dt_prov_profile.c
> +++ b/sys/dev/dt/dt_prov_profile.c
> @@ -101,15 +101,18 @@ dt_prov_profile_alloc(struct dt_probe *dtp, struct dt_softc *sc,
>  void
>  dt_clock(struct clockrequest *cr, void *cf, void *arg)
>  {
> -	uint64_t count, i;
> +	uint64_t count;
>  	struct dt_evt *dtev;
>  	struct dt_pcb *dp = arg;
>  
>  	count = clockrequest_advance(cr, dp->dp_nsecs);
> -	for (i = 0; i < count; i++) {
> -		dtev = dt_pcb_ring_get(dp, 1);
> -		if (dtev == NULL)
> -			return;
> -		dt_pcb_ring_consume(dp, dtev);
> -	}
> +	if (count == 0)
> +		return;
> +	else if (count > 1)
> +		dt_pcb_ring_drop(dp, count - 1);
> +
> +	dtev = dt_pcb_ring_get(dp, 1);
> +	if (dtev == NULL)
> +		return;
> +	dt_pcb_ring_consume(dp, dtev);
>  }
> diff --git a/sys/dev/dt/dtvar.h b/sys/dev/dt/dtvar.h
> index c6cfa7f66b3b..a1f6037e3233 100644
> --- a/sys/dev/dt/dtvar.h
> +++ b/sys/dev/dt/dtvar.h
> @@ -179,6 +179,7 @@ struct dt_pcb	*dt_pcb_alloc(struct dt_probe *, struct dt_softc *);
>  void		 dt_pcb_free(struct dt_pcb *);
>  void		 dt_pcb_purge(struct dt_pcb_list *);
>  
> +void		 dt_pcb_ring_drop(struct dt_pcb *, unsigned int);
>  struct dt_evt	*dt_pcb_ring_get(struct dt_pcb *, int);
>  void		 dt_pcb_ring_consume(struct dt_pcb *, struct dt_evt *);
>  
> -- 
> 2.34.1
> 



-- 
:wq Claudio