svn commit: r334752 - head/sys/kern

[Alan Cox]

Author: alc
Date: Thu Jun  7 02:54:11 2018
New Revision: 334752
URL: https://svnweb.freebsd.org/changeset/base/334752

Log:
  pidctrl_daemon() implements a variation on the classical, discrete PID
  controller that tries to handle early invocations of the controller,
  in other words, invocations before the expected end of the interval.
  However, there were some calculation errors in this early invocation
  case.  Notably, if an early invocation occurred while the error was
  negative, the derivative term was off by a large amount.  One visible
  effect of this error was that processes were being killed by the
  virtual memory system's OOM killer when in fact there was plentiful
  free memory.
  
  Correct a couple minor errors in the sysctl descriptions, and apply
  some style fixes.
  
  Reviewed by:	jeff, markj

Modified:
  head/sys/kern/subr_pidctrl.c

Modified: head/sys/kern/subr_pidctrl.c
==============================================================================
--- head/sys/kern/subr_pidctrl.c	Thu Jun  7 02:30:48 2018	(r334751)
+++ head/sys/kern/subr_pidctrl.c	Thu Jun  7 02:54:11 2018	(r334752)
@@ -59,14 +59,14 @@ pidctrl_init_sysctl(struct pidctrl *pc, struct sysctl_
 	    &pc->pc_olderror, 0, "Error value from last interval");
 	SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "integral", CTLFLAG_RD,
 	    &pc->pc_integral, 0, "Accumulated error integral (I)");
-	SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "derivative",
-	    CTLFLAG_RD, &pc->pc_derivative, 0, "Error derivative (I)");
+	SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "derivative", CTLFLAG_RD,
+	    &pc->pc_derivative, 0, "Error derivative (D)");
 	SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "input", CTLFLAG_RD,
 	    &pc->pc_input, 0, "Last controller process variable input");
 	SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "output", CTLFLAG_RD,
 	    &pc->pc_output, 0, "Last controller output");
 	SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "ticks", CTLFLAG_RD,
-	    &pc->pc_ticks, 0, "Last controler runtime");
+	    &pc->pc_ticks, 0, "Last controller runtime");
 	SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "setpoint", CTLFLAG_RW,
 	    &pc->pc_setpoint, 0, "Desired level for process variable");
 	SYSCTL_ADD_INT(NULL, parent, OID_AUTO, "interval", CTLFLAG_RD,
@@ -96,21 +96,20 @@ pidctrl_classic(struct pidctrl *pc, int input)
 	Kid = MAX(pc->pc_Kid, 1);
 	Kdd = MAX(pc->pc_Kdd, 1);
 
-	/* Compute P (proportional error), I (integral), D (derivative) */
+	/* Compute P (proportional error), I (integral), D (derivative). */
 	pc->pc_error = error;
 	pc->pc_integral =
 	    MAX(MIN(pc->pc_integral + error, pc->pc_bound), -pc->pc_bound);
 	pc->pc_derivative = error - pc->pc_olderror;
 
 	/* Divide by inverse gain values to produce output. */
-	output = ((pc->pc_error / Kpd) +
-	    (pc->pc_integral / Kid)) +
+	output = (pc->pc_error / Kpd) + (pc->pc_integral / Kid) +
 	    (pc->pc_derivative / Kdd);
 	/* Save for sysctl. */
 	pc->pc_output = output;
 	pc->pc_input = input;
 
-	return output;
+	return (output);
 }
 
 int
@@ -121,17 +120,18 @@ pidctrl_daemon(struct pidctrl *pc, int input)
 
 	error = pc->pc_setpoint - input;
 	/*
-	 * When ticks expired we reset our variables and start a new
+	 * When ticks expires we reset our variables and start a new
 	 * interval.  If we're called multiple times during one interval
 	 * we attempt to report a target as if the entire error came at
 	 * the interval boundary.
 	 */
-	if ((u_int)(ticks - pc->pc_ticks) >= pc->pc_interval) {
+	if ((u_int)ticks - pc->pc_ticks >= pc->pc_interval) {
 		pc->pc_ticks = ticks;
 		pc->pc_olderror = pc->pc_error;
 		pc->pc_output = pc->pc_error = 0;
 	} else {
-		error = MAX(error + pc->pc_error, 0);
+		/* Calculate the error relative to the last call. */
+		error -= pc->pc_error - pc->pc_output;
 	}
 
 	/* Fetch gains and prevent divide by zero. */
@@ -139,19 +139,19 @@ pidctrl_daemon(struct pidctrl *pc, int input)
 	Kid = MAX(pc->pc_Kid, 1);
 	Kdd = MAX(pc->pc_Kdd, 1);
 
-	/* Compute P (proportional error), I (integral), D (derivative) */
-	pc->pc_error = error;
+	/* Compute P (proportional error), I (integral), D (derivative). */
+	pc->pc_error += error;
 	pc->pc_integral =
 	    MAX(MIN(pc->pc_integral + error, pc->pc_bound), 0);
-	pc->pc_derivative = error - pc->pc_olderror;
+	pc->pc_derivative = pc->pc_error - pc->pc_olderror;
 
 	/* Divide by inverse gain values to produce output. */
-	output = ((error / Kpd) +
-	    (pc->pc_integral / Kid)) +
+	output = (error / Kpd) + (pc->pc_integral / Kid) +
 	    (pc->pc_derivative / Kdd);
 	output = MAX(output - pc->pc_output, 0);
+	/* Save for sysctl. */
 	pc->pc_output += output;
 	pc->pc_input = input;
 
-	return output;
+	return (output);
 }