Re: idle pool page gc
On Mon, Jan 19, 2015 at 11:28, David Gwynne wrote: if you're interested in seeing the effect of freeing at different intervals, you could try the diff below. it adds kern.pool.wait_free and kern.pool.wait_gc tunables so you can set how long a page has to be idle before a pool_put and the gc respectively will select a page for freeing. id prefer to pick values that Just Work(tm) all the time, but being able to test different values easily might be useful in the short term. ive been running the diff below in production for nearly a month now without issue. anyone want to ok this? if i commit this would anyone object? I thought the sysctl code was ok for testing, but not for commit. I don't think we should be adding more knobs like this. A diff without that would be much more attractive.
Re: idle pool page gc
On 23 Dec 2014, at 11:38, David Gwynne da...@gwynne.id.au wrote:
On Mon, Dec 22, 2014 at 10:54:16AM -0500, Ted Unangst wrote:
On Mon, Dec 22, 2014 at 14:59, Mike Belopuhov wrote:
1) how is it different from what we have now?
2) why can't you do it when you pool_put?
Right now, if you allocate a bunch of things, then free them, the
pages won't be freed because the timestamp is new. But you've already
freed everything, so there won't be a future pool_put to trigger the
final page freeing. This keeps pages from getting stuck.
3) why can't you call it from uvm when there's memory pressure since
from what i understand pool_reclaim was supposed to work like that?
Calling reclaim only when we're low on memory is sometimes too late.
and we never do. reclaim is only called when sysctl kern.pool_debug
is fiddled with.
4) i assume you don't want to call pool_reclaim from pool_gc_pages
because of the mutex_enter_try benefits, but why does logic in
these functions differ, e.g. why did you omit the pr_itemsperpage
bit?
We're not trying to release all free pages. Only the ones that are both
too many and too old. This is the same logic that is already in
pool_put.
7) it looks like pool_reclaim_all should also raise an IPL since it
does the same thing. wasn't it noteced before?
Likely. I don't think reclaim_all gets called very often.
or at all, really.
mikeb, id really appreciate it if you could benchmark with this diff.
if you're interested in seeing the effect of freeing at different
intervals, you could try the diff below. it adds kern.pool.wait_free
and kern.pool.wait_gc tunables so you can set how long a page has
to be idle before a pool_put and the gc respectively will select a
page for freeing.
id prefer to pick values that Just Work(tm) all the time, but being
able to test different values easily might be useful in the short
term.
ive been running the diff below in production for nearly a month now without
issue.
anyone want to ok this? if i commit this would anyone object?
dlg
Index: sbin/sysctl/sysctl.c
===
RCS file: /cvs/src/sbin/sysctl/sysctl.c,v
retrieving revision 1.207
diff -u -p -r1.207 sysctl.c
--- sbin/sysctl/sysctl.c 19 Nov 2014 18:04:54 - 1.207
+++ sbin/sysctl/sysctl.c 22 Dec 2014 23:44:52 -
@@ -44,6 +44,7 @@
#include sys/sched.h
#include sys/sensors.h
#include sys/vmmeter.h
+#include sys/pool.h
#include net/route.h
#include net/if.h
@@ -125,6 +126,7 @@ struct ctlname semname[] = CTL_KERN_SEMI
struct ctlname shmname[] = CTL_KERN_SHMINFO_NAMES;
struct ctlname watchdogname[] = CTL_KERN_WATCHDOG_NAMES;
struct ctlname tcname[] = CTL_KERN_TIMECOUNTER_NAMES;
+struct ctlname poolname[] = CTL_KERN_POOL_NAMES;
struct ctlname *vfsname;
#ifdef CTL_MACHDEP_NAMES
struct ctlname machdepname[] = CTL_MACHDEP_NAMES;
@@ -207,6 +209,7 @@ int sysctl_seminfo(char *, char **, int
int sysctl_shminfo(char *, char **, int *, int, int *);
int sysctl_watchdog(char *, char **, int *, int, int *);
int sysctl_tc(char *, char **, int *, int, int *);
+int sysctl_pool(char *, char **, int *, int, int *);
int sysctl_sensors(char *, char **, int *, int, int *);
void print_sensordev(char *, int *, u_int, struct sensordev *);
void print_sensor(struct sensor *);
@@ -388,6 +391,11 @@ parse(char *string, int flags)
return;
warnx(use dmesg to view %s, string);
return;
+ case KERN_POOL:
+ len = sysctl_pool(string, bufp, mib, flags, type);
+ if (len 0)
+ return;
+ break;
case KERN_PROC:
if (flags == 0)
return;
@@ -1633,6 +1641,7 @@ struct list semlist = { semname, KERN_SE
struct list shmlist = { shmname, KERN_SHMINFO_MAXID };
struct list watchdoglist = { watchdogname, KERN_WATCHDOG_MAXID };
struct list tclist = { tcname, KERN_TIMECOUNTER_MAXID };
+struct list poollist = { poolname, KERN_POOL_MAXID };
/*
* handle vfs namei cache statistics
@@ -2302,6 +2311,25 @@ sysctl_tc(char *string, char **bufpp, in
return (-1);
mib[2] = indx;
*typep = tclist.list[indx].ctl_type;
+ return (3);
+}
+
+/*
+ * Handle pools support
+ */
+int
+sysctl_pool(char *string, char **bufpp, int mib[], int flags, int *typep)
+{
+ int indx;
+
+ if (*bufpp == NULL) {
+ listall(string, poollist);
+ return (-1);
+ }
+ if ((indx = findname(string, third, bufpp, poollist)) == -1)
+ return (-1);
+ mib[2] = indx;
+ *typep = poollist.list[indx].ctl_type;
return (3);
}
Index: sys/kern/init_main.c
===
RCS file: /cvs/src/sys/kern/init_main.c,v
Re: massage volume control task
On 17 Jan 2015, at 11:06, Jim Smith j...@insec.sh wrote:
On 2 Jan 2015, at 4:15 pm, David Gwynne da...@gwynne.id.au wrote:
can someone test this?
it allocates storage for the volume change details rather than cast
arguments to a single global task.
adds some safety while there if audio0 is a hotplug device.
ok?
The keyboard audio controls on my X120e following -current work
same as before with this diff applied.
cool, thank you.
unless anyone objects im going to commit this tomorrow morning (tues 20th of
jan around 11am, gmt+10).
cheers,
dlg
Index: audio.c
===
RCS file: /cvs/src/sys/dev/audio.c,v
retrieving revision 1.125
diff -u -p -r1.125 audio.c
--- audio.c 19 Dec 2014 22:44:58 - 1.125
+++ audio.c 2 Jan 2015 06:08:39 -
@@ -465,11 +465,6 @@ audioattach(struct device *parent, struc
}
DPRINTF((audio_attach: inputs ports=0x%x, output ports=0x%x\n,
sc-sc_inports.allports, sc-sc_outports.allports));
-
-#if NWSKBD 0
-task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback, NULL,
-NULL);
-#endif /* NWSKBD 0 */
}
int
@@ -3432,27 +3427,39 @@ filt_audiowrite(struct knote *kn, long h
}
#if NWSKBD 0
+struct wskbd_vol_change {
+struct task t;
+long dir;
+long out;
+};
+
int
wskbd_set_mixervolume(long dir, long out)
{
struct audio_softc *sc;
+struct wskbd_vol_change *ch;
if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
DPRINTF((wskbd_set_mixervolume: audio_cd\n));
return (ENXIO);
}
-task_del(systq, sc-sc_mixer_task);
-task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback,
-(void *)dir, (void *)out);
-task_add(systq, sc-sc_mixer_task);
+ch = malloc(sizeof(*ch), M_TEMP, M_NOWAIT);
+if (ch == NULL)
+return (ENOMEM);
+
+task_set(ch-t, wskbd_set_mixervolume_callback, ch, NULL);
+ch-dir = dir;
+ch-out = out;
+task_add(systq, ch-t);
return (0);
}
void
-wskbd_set_mixervolume_callback(void *arg1, void *arg2)
+wskbd_set_mixervolume_callback(void *xch, void *null)
{
+struct wskbd_vol_change *ch = xch;
struct audio_softc *sc;
struct au_mixer_ports *ports;
mixer_devinfo_t mi;
@@ -3461,19 +3468,19 @@ wskbd_set_mixervolume_callback(void *arg
u_int gain;
int error;
-if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
-DPRINTF((%s: audio_cd\n, __func__));
-return;
-}
+dir = ch-dir;
+out = ch-out;
+free(ch, M_TEMP, sizeof(*ch));
-dir = (long)arg1;
-out = (long)arg2;
+sc = (struct audio_softc *)device_lookup(audio_cd, 0);
+if (sc == NULL)
+return;
ports = out ? sc-sc_outports : sc-sc_inports;
if (ports-master == -1) {
DPRINTF((%s: master == -1\n, __func__));
-return;
+goto done;
}
if (dir == 0) {
@@ -3482,7 +3489,7 @@ wskbd_set_mixervolume_callback(void *arg
error = au_get_mute(sc, ports, mute);
if (error != 0) {
DPRINTF((%s: au_get_mute: %d\n, __func__, error));
-return;
+goto done;
}
mute = !mute;
@@ -3490,7 +3497,7 @@ wskbd_set_mixervolume_callback(void *arg
error = au_set_mute(sc, ports, mute);
if (error != 0) {
DPRINTF((%s: au_set_mute: %d\n, __func__, error));
-return;
+goto done;
}
} else {
/* Raise or lower volume */
@@ -3499,7 +3506,7 @@ wskbd_set_mixervolume_callback(void *arg
error = sc-hw_if-query_devinfo(sc-hw_hdl, mi);
if (error != 0) {
DPRINTF((%s: query_devinfo: %d\n, __func__, error));
-return;
+goto done;
}
au_get_gain(sc, ports, gain, balance);
@@ -3512,8 +3519,11 @@ wskbd_set_mixervolume_callback(void *arg
error = au_set_gain(sc, ports, gain, balance);
if (error != 0) {
DPRINTF((%s: au_set_gain: %d\n, __func__, error));
-return;
+goto done;
}
}
+
+done:
+device_unref(sc-dev);
}
#endif /* NWSKBD 0 */
Re: [patch] siphash static functions
On 16 Jan 2015, at 20:45, Fritjof Bornebusch frit...@alokat.org wrote:
Hi tech@,
aren't these functions supposed to be static?
no, static functions in the kernel means the debugger cant see their names, and
we like seeing things in the debugger.
they probably should be static in userland though...
fritjof
Index: siphash.c
===
RCS file: /cvs/src/sys/crypto/siphash.c,v
retrieving revision 1.1
diff -u -p -r1.1 siphash.c
--- siphash.c 4 Nov 2014 03:01:14 - 1.1
+++ siphash.c 16 Jan 2015 10:41:37 -
@@ -48,8 +48,8 @@
#include crypto/siphash.h
-void SipHash_CRounds(SIPHASH_CTX *, int);
-void SipHash_Rounds(SIPHASH_CTX *, int);
+static void SipHash_CRounds(SIPHASH_CTX *, int);
+static void SipHash_Rounds(SIPHASH_CTX *, int);
void
SipHash_Init(SIPHASH_CTX *ctx, const SIPHASH_KEY *key)
@@ -147,7 +147,7 @@ SipHash(const SIPHASH_KEY *key, int rc,
#define SIP_ROTL(x, b) ((x) (b)) | ( (x) (64 - (b)))
-void
+static void
SipHash_Rounds(SIPHASH_CTX *ctx, int rounds)
{
while (rounds--) {
@@ -171,7 +171,7 @@ SipHash_Rounds(SIPHASH_CTX *ctx, int rou
}
}
-void
+static void
SipHash_CRounds(SIPHASH_CTX *ctx, int rounds)
{
u_int64_t m = lemtoh64((u_int64_t *)ctx-buf);
