Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 5 Jul 2018 15:27:38 +0000 (UTC)
From:      Kyle Evans <kevans@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r335984 - stable/11/sys/kern
Message-ID:  <201807051527.w65FRcgA007922@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kevans
Date: Thu Jul  5 15:27:38 2018
New Revision: 335984
URL: https://svnweb.freebsd.org/changeset/base/335984

Log:
  MFC r335479, r335509
  
  MFC r335479: subr_hints: simplify a little bit
  
  Some complexity exists in these bits that isn't needed. The sysctl handler,
  upon change to '2', runs through the current set of hints and sets them in
  the kenv.
  
  However, this isn't at all necessary if we're pulling hints from the kenv,
  static or dynamic, as the former will get added to the latter in
  init_dynamic_kenv (see: kern_environment.c). We can reduce this
  configuration to just adding static_hints to the kenv if we were previously
  using them.
  
  The changes in res_find are minimal and based on the observation that once
  use_kenv gets set to '1' it will never be reset to '0', and it gets set to
  '1' as soon as we hit fallback mode. Later work will refactor res_find a
  little bit and eliminate this now-local, because it's become clear that
  there's some funkiness revolving around use_kenv=1 and it being used to
  imply that we're certainly looking at the dynamic_kenv.
  
  MFC r335509: subr_hints: Fix acpi unit hinting (at the very least)
  
  The refactoring in r335479 overlooked the fact that the dynamic kenv can
  also be switched to if hintmode == 0. This is problematic because the
  checkmethod bits are only ever ran once, but it worked previously because
  the use_kenv was a global state and the first lookup would enable it if
  occurring after the dynamic environment has been setup.
  
  Extending our local definition of use_kenv to include all non-STATIC
  hintmodes as long as the dynamic_kenv is setup fixes this. We still have
  potential issues if the dynamic kenv comes up while we're doing an anchored
  search through the environment, but this is not much of a concern right now
  because:
  
  1.) The dynamic environment comes up super early in boot, just after kmem
  
  2.) This is going to get rewritten to provide a safer mechanism for the
  anchored searches, ensuring that we continue using the same environment
  chain (dynamic env or static fallback) for all anchored search invocations

Modified:
  stable/11/sys/kern/subr_hints.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/kern/subr_hints.c
==============================================================================
--- stable/11/sys/kern/subr_hints.c	Thu Jul  5 14:12:56 2018	(r335983)
+++ stable/11/sys/kern/subr_hints.c	Thu Jul  5 15:27:38 2018	(r335984)
@@ -35,12 +35,15 @@ __FBSDID("$FreeBSD$");
 #include <sys/systm.h>
 #include <sys/bus.h>
 
+#define	HINTMODE_KENV		0
+#define	HINTMODE_STATIC		1
+#define	HINTMODE_FALLBACK	2
+
 /*
  * Access functions for device resources.
  */
 
 static int checkmethod = 1;
-static int use_kenv;
 static char *hintp;
 
 /*
@@ -54,10 +57,8 @@ sysctl_hintmode(SYSCTL_HANDLER_ARGS)
 {
 	const char *cp;
 	char *line, *eq;
-	int eqidx, error, from_kenv, i, value;
+	int eqidx, error, i, value;
 
-	from_kenv = 0;
-	cp = kern_envp;
 	value = hintmode;
 
 	/* Fetch candidate for new hintmode value */
@@ -65,47 +66,33 @@ sysctl_hintmode(SYSCTL_HANDLER_ARGS)
 	if (error || req->newptr == NULL)
 		return (error);
 
-	if (value != 2)
+	if (value != HINTMODE_FALLBACK)
 		/* Only accept swithing to hintmode 2 */
 		return (EINVAL);
 
-	/* Migrate from static to dynamic hints */
-	switch (hintmode) {
-	case 0:
-		if (dynamic_kenv) {
-			/*
-			 * Already here. But assign hintmode to 2, to not
-			 * check it in the future.
-			 */
-			hintmode = 2;
-			return (0);
-		}
-		from_kenv = 1;
-		cp = kern_envp;
-		break;
-	case 1:
-		cp = static_hints;
-		break;
-	case 2:
-		/* Nothing to do, hintmode already 2 */
+	/*
+	 * The rest of the sysctl handler is just making sure that our
+	 * environment is consistent with the world we've already seen.
+	 * If we came from kenv at all, then we have nothing to do: static
+	 * kenv will get merged into dynamic kenv as soon as kmem becomes
+	 * available, dynamic kenv is the environment we'd be setting these
+	 * things in anyways. Therefore, we have nothing left to do unless
+	 * we came from a static hints configuration.
+	 */
+	if (hintmode != HINTMODE_STATIC) {
+		hintmode = value;
 		return (0);
 	}
 
-	while (cp) {
-		i = strlen(cp);
-		if (i == 0)
-			break;
-		if (from_kenv) {
-			if (strncmp(cp, "hint.", 5) != 0)
-				/* kenv can have not only hints */
-				continue;
-		}
+	cp = static_hints;
+	while (cp && *cp != '\0') {
 		eq = strchr(cp, '=');
 		if (eq == NULL)
 			/* Bad hint value */
 			continue;
 		eqidx = eq - cp;
 
+		i = strlen(cp);
 		line = malloc(i+1, M_TEMP, M_WAITOK);
 		strcpy(line, cp);
 		line[eqidx] = '\0';
@@ -115,7 +102,6 @@ sysctl_hintmode(SYSCTL_HANDLER_ARGS)
 	}
 
 	hintmode = value;
-	use_kenv = 1;
 	return (0);
 }
 
@@ -135,7 +121,7 @@ res_find(int *line, int *startln,
 {
 	int n = 0, hit, i = 0;
 	char r_name[32];
-	int r_unit;
+	int r_unit, use_kenv = (hintmode != HINTMODE_STATIC && dynamic_kenv);
 	char r_resname[32];
 	char r_value[128];
 	const char *s, *cp;
@@ -145,13 +131,13 @@ res_find(int *line, int *startln,
 		hintp = NULL;
 
 		switch (hintmode) {
-		case 0:		/* loader hints in environment only */
+		case HINTMODE_KENV:	/* loader hints in environment only */
 			break;
-		case 1:		/* static hints only */
+		case HINTMODE_STATIC:	/* static hints only */
 			hintp = static_hints;
 			checkmethod = 0;
 			break;
-		case 2:		/* fallback mode */
+		case HINTMODE_FALLBACK:		/* fallback mode */
 			if (dynamic_kenv) {
 				mtx_lock(&kenv_lock);
 				cp = kenvp[0];



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201807051527.w65FRcgA007922>