Date: Fri, 25 Jun 2010 11:31:17 +0300 From: Andriy Gapon <avg@freebsd.org> To: Alexander Motin <mav@freebsd.org>, freebsd-acpi@freebsd.org Subject: Re: cpufreq_curr_sysctl: memory allocation Message-ID: <4C246955.9000700@freebsd.org> In-Reply-To: <4C1E3FA4.1020009@freebsd.org> References: <4C1E3AE9.3020505@freebsd.org> <4C1E3D44.6020708@FreeBSD.org> <4C1E3FA4.1020009@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
on 20/06/2010 19:19 Andriy Gapon said the following: > I agree, but I hope that a single buffer should be sufficient. > As I understand, all sysctl operate under Giant unless specifically flagged > otherwise. And I don't see any code to explicitly handle concurrent invocations > in cpufreq_curr_sysctl. > My take on it. Seems to work without problems. diff --git a/sys/kern/kern_cpu.c b/sys/kern/kern_cpu.c index 4c4f961..3429a0f 100644 --- a/sys/kern/kern_cpu.c +++ b/sys/kern/kern_cpu.c @@ -76,6 +76,7 @@ struct cpufreq_softc { device_t dev; struct sysctl_ctx_list sysctl_ctx; struct task startup_task; + struct cf_level *levels_buf; }; struct cf_setting_array { @@ -180,6 +181,8 @@ cpufreq_attach(device_t dev) CF_DEBUG("initializing one-time data for %s\n", device_get_nameunit(dev)); + sc->levels_buf = malloc(CF_MAX_LEVELS * sizeof(*sc->levels_buf), + M_DEVBUF, M_WAITOK); SYSCTL_ADD_PROC(&sc->sysctl_ctx, SYSCTL_CHILDREN(device_get_sysctl_tree(parent)), OID_AUTO, "freq", CTLTYPE_INT | CTLFLAG_RW, sc, 0, @@ -227,6 +230,7 @@ cpufreq_detach(device_t dev) numdevs = devclass_get_count(cpufreq_dc); if (numdevs == 1) { CF_DEBUG("final shutdown for %s\n", device_get_nameunit(dev)); + free(sc->levels_buf, M_DEVBUF); } return (0); @@ -870,9 +874,7 @@ cpufreq_curr_sysctl(SYSCTL_HANDLER_ARGS) devs = NULL; sc = oidp->oid_arg1; - levels = malloc(CF_MAX_LEVELS * sizeof(*levels), M_TEMP, M_NOWAIT); - if (levels == NULL) - return (ENOMEM); + levels = sc->levels_buf; error = CPUFREQ_GET(sc->dev, &levels[0]); if (error) @@ -915,8 +917,6 @@ cpufreq_curr_sysctl(SYSCTL_HANDLER_ARGS) out: if (devs) free(devs, M_TEMP); - if (levels) - free(levels, M_TEMP); return (error); } @@ -934,7 +934,7 @@ cpufreq_levels_sysctl(SYSCTL_HANDLER_ARGS) /* Get settings from the device and generate the output string. */ count = CF_MAX_LEVELS; - levels = malloc(count * sizeof(*levels), M_TEMP, M_NOWAIT); + levels = sc->levels_buf; if (levels == NULL) { sbuf_delete(&sb); return (ENOMEM); @@ -957,7 +957,6 @@ cpufreq_levels_sysctl(SYSCTL_HANDLER_ARGS) error = sysctl_handle_string(oidp, sbuf_data(&sb), sbuf_len(&sb), req); out: - free(levels, M_TEMP); sbuf_delete(&sb); return (error); } -- Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C246955.9000700>