Re: svn commit: r329882 - in head/sys: conf kern sys vm

[Ed Schouten]

Hi Jeff,

2018-02-23 23:51 GMT+01:00 Jeff Roberson :
>   Add a generic Proportional Integral Derivative (PID) controller algorithm 
> and
>   use it to regulate page daemon output.

That looks pretty nifty. Looking at the code, it exposes metrics
through sysctl. Be sure to give prometheus_sysctl_exporter(8) a try if
you ever want to graph the characteristics of the controller!

-- 
Ed Schouten 
Nuxi, 's-Hertogenbosch, the Netherlands
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r329882 - in head/sys: conf kern sys vm

[Pieter de Goeje]

Hi,

I'm curious, it looks like Kd is quite significant by default, I assume 
to limit the slew rate of the output.


Given that the error is an integer, and pidctrl_daemon() is called at 
irregular intervals, it seems to me that the derivative will be very noisy.


In my experience with PID controllers controlling physical processes, 
one would need some kind of filtering on the input to make Kd useful, 
otherwise it would just contribute more instability to the system. In 
practice even a simple exponential (single pole) filter can help a lot 
at the cost of increased response time.


Is this something that was considered?

- Pieter de Goeje
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r329882 - in head/sys: conf kern sys vm

[Jeff Roberson]

Author: jeff
Date: Fri Feb 23 22:51:51 2018
New Revision: 329882
URL: https://svnweb.freebsd.org/changeset/base/329882

Log:
  Add a generic Proportional Integral Derivative (PID) controller algorithm and
  use it to regulate page daemon output.
  
  This provides much smoother and more responsive page daemon output, 
anticipating
  demand and avoiding pageout stalls by increasing the number of pages to match
  the workload.  This is a reimplementation of work done by myself and mlaier at
  Isilon.
  
  Reviewed by:  bsdimp
  Tested by:pho
  Sponsored by: Netflix, Dell/EMC Isilon
  Differential Revision:https://reviews.freebsd.org/D14402

Added:
  head/sys/kern/subr_pidctrl.c   (contents, props changed)
  head/sys/sys/pidctrl.h   (contents, props changed)
Modified:
  head/sys/conf/files
  head/sys/vm/vm_meter.c
  head/sys/vm/vm_page.c
  head/sys/vm/vm_pageout.c
  head/sys/vm/vm_pagequeue.h

Modified: head/sys/conf/files
==
--- head/sys/conf/files Fri Feb 23 22:23:28 2018(r329881)
+++ head/sys/conf/files Fri Feb 23 22:51:51 2018(r329882)
@@ -3874,6 +3874,7 @@ kern/subr_msgbuf.cstandard
 kern/subr_param.c  standard
 kern/subr_pcpu.c   standard
 kern/subr_pctrie.c standard
+kern/subr_pidctrl.cstandard
 kern/subr_power.c  standard
 kern/subr_prf.cstandard
 kern/subr_prof.c   standard

Added: head/sys/kern/subr_pidctrl.c
==
--- /dev/null   00:00:00 1970   (empty, because file is newly added)
+++ head/sys/kern/subr_pidctrl.cFri Feb 23 22:51:51 2018
(r329882)
@@ -0,0 +1,157 @@
+/*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
+ * Copyright (c) 2017,  Jeffrey Roberson 
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice unmodified, this list of conditions, and the following
+ *disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include 
+__FBSDID("$FreeBSD$");
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+void
+pidctrl_init(struct pidctrl *pc, int interval, int setpoint, int bound,
+int Kpd, int Kid, int Kdd)
+{
+
+   bzero(pc, sizeof(*pc));
+   pc->pc_setpoint = setpoint;
+   pc->pc_interval = interval;
+   pc->pc_bound = bound * setpoint * Kid;
+   pc->pc_Kpd = Kpd;
+   pc->pc_Kid = Kid;
+   pc->pc_Kdd = Kdd;
+}
+
+void
+pidctrl_init_sysctl(struct pidctrl *pc, struct sysctl_oid_list *parent)
+{
+
+   SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "error", CTLFLAG_RD,
+   >pc_error, 0, "Current difference from setpoint value (P)");
+   SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "olderror", CTLFLAG_RD,
+   >pc_olderror, 0, "Error value from last interval");
+   SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "integral", CTLFLAG_RD,
+   >pc_integral, 0, "Accumulated error integral (I)");
+   SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "derivative",
+   CTLFLAG_RD, >pc_derivative, 0, "Error derivative (I)");
+   SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "input", CTLFLAG_RD,
+   >pc_input, 0, "Last controller process variable input");
+   SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "output", CTLFLAG_RD,
+   >pc_output, 0, "Last controller output");
+   SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "ticks", CTLFLAG_RD,
+   >pc_ticks, 0, "Last controler runtime");
+   SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "setpoint", CTLFLAG_RW,
+   >pc_setpoint, 0, "Desired level for process variable");
+   SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "interval", CTLFLAG_RD,
+   >pc_interval, 0, "Interval between calculations (ticks)");
+