From: Christian Ludwig <cludwig@genua.de>
Subject: Re: dt(4) memory tweaks
To: Martin Pieuchot <mpi@grenadille.net>
Cc: <tech@openbsd.org>
Date: Tue, 5 Nov 2024 07:39:01 +0100

On Sat, Nov 02, 2024 at 06:18:32PM +0100 Martin Pieuchot wrote:
> softintr_establish() allocates memory, even if doesn't sleep I'd like to
> keep all allocations in the same place.
> 
> Use M_DEVBUF consistently for all memory chunks in the softc.
> 
> No need to check for NULL before calling free(9).
> 
> Make spacing coherent with the rest of the file when arguments passed to
> a function do not fit in a line.

Looks good. Ok from me, FWIW.

> Index: dev/dt/dt_dev.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/dt/dt_dev.c,v
> diff -u -p -r1.39 dt_dev.c
> --- dev/dt/dt_dev.c	2 Nov 2024 17:03:12 -0000	1.39
> +++ dev/dt/dt_dev.c	2 Nov 2024 17:09:42 -0000
> @@ -206,11 +206,6 @@ dtopen(dev_t dev, int flags, int mode, s
>  	TAILQ_INIT(&sc->ds_pcbs);
>  	sc->ds_lastcpu = 0;
>  	sc->ds_evtcnt = 0;
> -	sc->ds_si = softintr_establish(IPL_SOFTCLOCK, dt_deferred_wakeup, sc);
> -	if (sc->ds_si == NULL) {
> -		dtfree(sc);
> -		return ENOMEM;
> -	}
>  
>  	SLIST_INSERT_HEAD(&dtdev_list, sc, ds_next);
>  
> @@ -233,7 +228,6 @@ dtclose(dev_t dev, int flags, int mode, 
>  	SLIST_REMOVE(&dtdev_list, sc, dt_softc, ds_next);
>  	dt_ioctl_record_stop(sc);
>  	dt_pcb_purge(&sc->ds_pcbs);
> -	softintr_disestablish(sc->ds_si);
>  	dtfree(sc);
>  
>  	return 0;
> @@ -364,8 +358,8 @@ dtalloc(void)
>  		return NULL;
>  
>  	for (i = 0; i < ncpusfound; i++) {
> -		dtev = mallocarray(DT_EVTRING_SIZE, sizeof(*dtev), M_DT,
> -				   M_WAITOK|M_CANFAIL|M_ZERO);
> +		dtev = mallocarray(DT_EVTRING_SIZE, sizeof(*dtev), M_DEVBUF,
> +		    M_WAITOK|M_CANFAIL|M_ZERO);
>  		if (dtev == NULL)
>  			break;
>  		sc->ds_cpu[i].dc_ring = dtev;
> @@ -375,6 +369,12 @@ dtalloc(void)
>  		return NULL;
>  	}
>  
> +	sc->ds_si = softintr_establish(IPL_SOFTCLOCK, dt_deferred_wakeup, sc);
> +	if (sc->ds_si == NULL) {
> +		dtfree(sc);
> +		return NULL;
> +	}
> +
>  	return sc;
>  }
>  
> @@ -384,10 +384,12 @@ dtfree(struct dt_softc *sc)
>  	struct dt_evt *dtev;
>  	int i;
>  
> +	if (sc->ds_si != NULL)
> +		softintr_disestablish(sc->ds_si);
> +
>  	for (i = 0; i < ncpusfound; i++) {
>  		dtev = sc->ds_cpu[i].dc_ring;
> -		if (dtev != NULL)
> -			free(dtev, M_DT, DT_EVTRING_SIZE * sizeof(*dtev));
> +		free(dtev, M_DEVBUF, DT_EVTRING_SIZE * sizeof(*dtev));
>  	}
>  	free(sc, M_DEVBUF, sizeof(*sc));
>  }