Changeset 76

Timestamp:

05/31/05 00:59:47 (4 years ago)

Author:

jtv

Message:

Unified logging; revamped free-space estimation algorithm

Files:

• TODO (modified) (1 diff)
• debian/changelog (modified) (1 diff)
• src/Makefile (modified) (2 diffs)
• src/log.c (modified) (3 diffs)
• src/log.h (modified) (1 diff)
• src/main.c (modified) (4 diffs)
• src/memory.c (modified) (10 diffs)
• src/memory.h (modified) (1 diff)
• src/opts.c (modified) (1 diff)
• src/swaps.c (modified) (23 diffs)
• src/swaps.h (modified) (1 diff)

TODO

@@ -1 @@
-
-
+
+
+
-
-Prettiness
- - Unify logging/printing code: print-only or dynamic choice
-
+Policy Weaknesses
+
+Current policy is a bit weak when it comes to filesystem caching.  The problem
+is that cache populations grow and grow until they hit the limit of available
+physical memory--creating an arguably false impression of memory shortage.  The
+flexibility of the Linux virtual memory subsystem makes this, as far as I can
+see at this stage, impossible to diagnose directly.
+
+Most noticeable symptom: existing swapfiles are freed up sometime between system
+boot (when little data is cached and most physical memory is still free) and the
+moment when ever-growing caches push the percentage of free memory below our
+daemon's allocation threshold.  We end up allocating a new swapfile shortly
+after deallocating our last one, just as the user is getting into the swing of
+things and expects his system to respond smoothly.  While we're setting up that
+new swapfile, it doesn't.  This is hard to anticipate; starting up in bull mode
+may help as a practical matter by deferring the decision to deallocate, but
+only if the associated cooldown period is long enough for filesystem caches to
+fill up again and bring the system into a "steady state."
+
+There are other cases that can trigger this behaviour, though.  Consider this
+situation: some application X eats up huge amounts of virtual memory, most of
+which is in active use and therefore resides in physical memory.  Swap is full
+of rarely-used virtual memory pages.  Little or no filesystem caching occurs.
+Next, X finishes and suddenly we have lots of free physical memory--although
+swap remains densely populated.  I'll need to do some math (probably quite
+simple, once I have the right formula) to figure out what can and cannot happen
+here.
+
+Scenario A: if most of the swapped-out data is touched again before we go into
+bear mode and consider deallocating, we're fine.  We'll end up freeing largely
+unused swap space.
+
+Scenario B: if swap is deallocated at this stage, any remaining data it held
+will have to be read back in a hurry--plus, continued system operation may be
+suboptimal because some of that data is "colder" than filesystem data that could
+have been cached in the physical memory that X left free in its wake.
+
+Scenario C: if the "data vacuum" left by X is filled up by filesystem caches, we
+may end up thinking (once again) that we're short on memory--and allocating swap
+space that we won't be needing at all.  Bull mode will be prolonged when a
+return to bear mode is in order.
+
+One consolation: since filesystem caches are never swapped to disk, they cannot
+exceed physical memory size.  This means that the algorithm, for any acceptable
+set of parameters, can "overshoot" by at most a single swapfile.
+
+We could try to distinguish "false shortages" from real memory needs by
+ensuring that there is some "tripwire swap"--perhaps an existing conventional
+swap partition, or a special swapfile numbered zero.  An increase in the
+population of this swap would indicate true memory shortage.  The only problem
+is that the system may not resort to using the tripwire swap until it is already
+desperate!  Can we set a positive priority to make the tripwire slightly more
+sensitive?  We'd also have to clean this file up from time to time, because once
+it fills up the mechanism breaks down.
+
+
+Bull/Bear Mode
+
+Currently, the program goes into bear mode if swapfile allocation fails and into
+bull mode when swapfiles are allocated.  In the steady state we're somewhere
+inbetween.  Maybe this isn't the best way to do it.
+
+What if we inhibited deallocations not just in bull mode, but instead required
+that the percentage of available space was above the deallocation limit for an
+entire cooldown period?
+

debian/changelog

@@ +1 @@
+swapspace (1.3) unstable; urgency=low
+
+  * Consider part of non-dirty cached memory to be free space
+  * Unified logging code--it's not pretty, but it's prettier
+  * No longer bother with "negotiable" minimum swapfile sizes
+  * Clarified startup message ("require n extra bytes" to "would prefer []")
+  * Documented existing policy weaknesses, possible solutions
+  * Systematic rounding of swap sizes to page size
+
+ -- Jeroen T. Vermeulen <jtv@sipa.or.th>  Thu, 26 May 2005 17:00:00 +0700
+
-swapspace (1.2) unstable; urgency=low
-
-  * Initial Debian package.
-
-y
+i
-

src/Makefile

@@ -16 +16 @@
-hog : hog.o
-
-
+ memory.h
-
-
+log.h 
-
-
+log.h 
-
-opts.o : opts.c opts.h main.h
@@ -26 +26 @@
-support.o : support.c config.h env.h support.h
-
-
+log.h main.h memory.h 
-
-clean :

src/log.c

@@ -16 +16 @@
-Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
-*/
+#include <errno.h>
+#include <stdio.h>
+#include <string.h>
+
-#include "log.h"
-
-static bool logging = false;
-
-
-#ifndef LOG_PERROR
@@ -37 +40 @@
-
-
-bool am_logging(void)
-{
-  return logging;
-}
-
-
-/// Stop logging, e.g. because storage containing log name goes out of scope
-void log_close()
@@ -50 +47 @@
-}
-
+
+static const char *prefix(int priority)
+{
+  switch (priority)
+  {
+  case LOG_ERR:         return "Error: ";
+  case LOG_WARNING:     return "Warning: ";
+  case LOG_NOTICE:      return "Notice: ";
+  }
+  return "";
+}
+
+
+static FILE *stream(int priority)
+{
+  switch (priority)
+  {
+  case LOG_ERR:
+  case LOG_WARNING:
+    return stderr;
+  }
+  return stdout;
+}
+
+
+void log_msg(int priority, const char msg[])
+{
+  if (logging) syslog(priority, "%s", msg);
+  else         fprintf(stream(priority), "%s%s\n", prefix(priority), msg);
+}
+
+
+void log_perr(int priority, const char msg[])
+{
+  log_perrno(priority, msg, errno);
+}
+
+
+void log_perrno(int priority, const char msg[], int fault_errno)
+{
+  if (!fault_errno)
+    log_msg(priority, msg);
+  else
+  {
+    const char *std = strerror(fault_errno);
+    if (logging) syslog(priority, "%s: %s", msg, std);
+    else fprintf(stream(priority), "%s%s: %s\n", prefix(priority), msg, std);
+  }
+}
+
+
+void log_str(int priority, const char msg[], const char arg[])
+{
+  if (logging) syslog(priority, "%s '%s'", msg, arg);
+  else fprintf(stream(priority), "%s%s '%s'\n", prefix(priority), msg, arg);
+}
+
+
+void log_perr_str(int priority, const char msg[], const char arg[])
+{
+  const int fault_errno = errno;
+  if (!fault_errno)
+  {
+    log_str(priority, msg, arg);
+  }
+  else
+  {
+    const char *std = strerror(fault_errno);
+    if (logging)
+      syslog(priority, "%s '%s': %s", msg, arg, std);
+    else
+      fprintf(stream(priority),"%s%s '%s': %s\n",prefix(priority),msg,arg,std);
+  }
+}
+
+
+void log_int(int priority, const char msg[], int arg)
+{
+  if (logging) syslog(priority, "%s, %d", msg, arg);
+  else fprintf(stream(priority), "%s%s %d\n", prefix(priority), msg, arg);
+}
+
+
+void log_discrep(int priority,
+    const char msg[],
+    int swapfile,
+    memsize_t expected,
+    memsize_t found)
+{
+  if (logging)
+    syslog(priority,
+        "Discrepancy in swapfile %d: %s (%lld bytes vs. %lld)",
+        swapfile,
+        msg,
+        expected,
+        found);
+  else
+    fprintf(stream(priority),
+        "%sDiscrepancy in swapfile %d: %s (%lld bytes vs. %lld)\n",
+        prefix(priority),
+        swapfile,
+        msg,
+        expected,
+        found);
+}
+

src/log.h

@@ -22 +22 @@
-
-#include "main.h"
+#include "memory.h"
-
-/// Open log, with given program name (string storage must remain available!)
-void log_start(const char myname[]);
-
-/// Has logging been enabled?  If not, try writing any messages to stderr
-bool am_logging(void);
-
-/// Stop logging, e.g. because storage containing log name goes out of scope
-void log_close();
-
+/// Write plain message to log, or standard output/error as appropriate
+void log_msg(int priority, const char msg[]);
+
+/// Log plain message followed by quoted string
+void log_str(int priority, const char msg[], const char arg[]);
+
+/// Log message in perror() style (i.e. with errno-based message appended)
+void log_perr(int priority, const char msg[]);
+
+/// Log message with given errno (or ignore error number if it is zero)
+void log_perrno(int priority, const char msg[], int fault_errno);
+
+/// Log message, perror()-style, but appending a quoted string
+void log_perr_str(int priority, const char msg[], const char arg[]);
+
+/// Log message, but appending an integer argument
+void log_int(int priority, const char msg[], int arg);
+
+/// Log logfile size discrepancy
+void log_discrep(int priority,
+    const char msg[],
+    int swapfile,
+    memsize_t expected,
+    memsize_t found);
+
-#endif
-

src/main.c

@@ -78 +78 @@
-static volatile bool print_status=false;
-
+/// Was an immediate adjustment requested?
-static volatile bool adjust_swap=false;
-
@@ -94 +95 @@
-  if (unlikely(write(pidfd, localbuf, len) < len))
-  {
-fprintf(stderr, "Could not write pidfile %s: %s\n",pidfile,strerror(errno)
+log_perr_str(LOG_ERR, "Could not write pidfile", pidfile
-    return false;
-  }
@@ -114 +115 @@
-  if (unlikely(pidfd == -1))
-  {
-
-
-
-
-
+
+
-    else
-
-
-
-
+
-    return false;
-  }
@@ -207 +202 @@
-static void handle_requirements(void)
-{
-
-
-
-
+
+
+
-}
-

src/memory.c

@@ -39 +39 @@
-static int freetarget=25;
-
+// TODO: Make this adaptive based on actual cache usage, to avoid thrashing?
+/// Configuration item: what percentage of cache space do we consider "free"?
+static int cache_meekness=50;
+
-char *set_freetarget(long long pct)
-{
@@ -56 +60 @@
-  return NULL;
-}
+
+char *set_cache_meekness(long long pct)
+{
+  cache_meekness = (int)pct;
+  return NULL;
+}
+
-
-bool memory_check_config(void)
@@ -147 +158 @@
-  memsize_t MemTotal,
-            MemFree,
+            Cached,
+            Dirty,
+            Writeback,
+            SwapCached,
-            SwapTotal,
-            SwapFree;
@@ -156 +171 @@
-  switch (inf->entry[0])
-  {
+  case 'C':
+    if (strcmp(inf->entry, "Cached:")==0)          st->Cached = inf->value;
+    break;
+  case 'D':
+    if (strcmp(inf->entry, "Dirty:")==0)           st->Dirty = inf->value;
+    break;
-  case 'M':
-
-
+
+
+
+
+
-    break;
-  case 'S':
-
-
+
+
+
+
+
+
+
+
+
-    break;
-  }
@@ -173 +204 @@
-  if (unlikely(!fp))
-  {
-
-
-
-
+
-    return false;
-  }
@@ -183 +211 @@
-  while (read_meminfo_item(fp, &inf)) imbibe_meminfo_entry(&inf, result);
-  fclose(fp);
-
-
-
-
-
-
-
-
-
-
-
-
-
+
-  
-  return true;
@@ -207 +223 @@
-  {
-    printf("Initial memory status: ");
-require
+would prefer
-    else if (init_req < 0) printf("%lld bytes to spare\n", -init_req);
-  }
@@ -214 +230 @@
-
-
+/// How much cache space can we expect the system to free up?
+static inline memsize_t cache_free(const struct memstate *st)
+{
+  const memsize_t cache = st->Cached - (st->Dirty + st->Writeback);
+  return (cache > 0) ? (cache/100)*cache_meekness : 0;
+}
+
+
-static inline memsize_t space_free(const struct memstate *st)
-{
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-}
-
@@ -241 +290 @@
-  assert(upper_freelimit > lower_freelimit);
-
-
+, 0, 0, 0, 0
-  if (unlikely(!read_proc_meminfo(&st))) return MEMSIZE_ERROR;
-
@@ -248 +297 @@
-
-  // Policy: once we hit either freelimit, steer for "freetarget"% free space
-
+?
-  if (freepct < lower_freelimit || freepct > upper_freelimit)
-  {

src/memory.h

@@ -40 +40 @@
-char *set_upper_freelimit(long long pct);
-char *set_freetarget(long long pct);
+char *set_cache_meekness(long long pct);
-
-bool memory_check_config(void);

src/opts.c

@@ -61 +61 @@
-static const struct option options[] =
-{
+  { "cache_meekness",   'w', at_num,  0, 100, set_cache_meekness,
+  "Consider n% of cache memory to be free space" },
-  { "configfile",       'c', at_str,  1, PATH_MAX, set_configfile,
-  "Use configuration file s" },

src/swaps.c

@@ -61 +61 @@
-
-/// Smallest allowed swapfile size
-12*PAGE_SIZE
+(12*PAGE_SIZE) & PAGE_MASK
-/// Largest allowed swapfile size
-/** Don't set this too low.  The program will learn if it runs into file size
- * limits.
- */
-
+ & PAGE_MASK
-
-
@@ -100 +100 @@
-char *set_min_swapsize(long long size)
-{
-memsize_t)size
+(memsize_t)size & PAGE_MASK)
-  return NULL;
-}
@@ -106 +106 @@
-char *set_max_swapsize(long long size)
-{
-memsize_t)size
+(memsize_t)size) & PAGE_MASK
-  return NULL;
-}
@@ -119 +119 @@
-bool swaps_check_config(void)
-{
-  assert(!am_logging());
-  CHECK_CONFIG_ERR(min_swapsize > max_swapsize);
-
-  if (chdir(swappath) == -1)
-  {
-fprintf(stderr, "Could not cd to %s: %s\n", swappath, strerror(errno)
+log_perr_str(LOG_ERR, "Coult not cd to", swappath
-    return false;
-  }
-  if (!getcwd(swappath, sizeof(swappath)))
-  {
-fputs("Swap path too long\n", stderr
+log_msg(LOG_ERR, "Swap path too long"
-    return false;
-  }
@@ -141 +140 @@
-  for (int i=swappath_len-1; i >= 0; --i) if (isspace(swappath[i]))
-  {
-fputs("Not supported: swap path contains whitespace\n", stderr
+log_msg(LOG_ERR, "Not supported: swap path contains whitespace"
-    return false;
-  }
@@ -235 +234 @@
-  runcommand("mkswap", file);
-  const bool ok = (swapon(file, 0) == 0);
-
-
-
-
-
+
-  return ok;
-}
@@ -270 +265 @@
-  {
-    const int err = errno;
-
-
+
-    bytes -= block;
-    switch (err)
@@ -277 +271 @@
-    case EFBIG:
-      // File is too large.  Remember how much we were allowed to write.
-
+
+
-      break;
-    case ENOSPC:
@@ -299 +294 @@
-/**
- * @param filename File to be created
-
-
+
- * @return Size of new swapfile, which may differ from requested size.  Zero or
- * less indicates failure, in which case the file is deleted.  Negative result
@@ -306 +300 @@
- * likely to be appreciated.
- */
-, memsize_t min
+
-{
-  assert(min_swapsize <= max_swapsize);
+  assert(!(max_swapsize & ~PAGE_MASK));
+  assert(!(min_swapsize & ~PAGE_MASK));
+  assert(!(size & ~PAGE_MASK));
+
+  memsize_t realsize;
+
+  if (unlikely(size < min_swapsize)) return 0;
-
-  // Don't allocate swap for a while--we had a bad experience recently
-  if (unlikely(cooldown_bear) && in_cooldown()) return 0;
-
-
-
-
-
+
-
-  // TODO: Think about reuse of existing swapfiles
@@ -323 +321 @@
-  if (unlikely(fd == -1))
-  {
-
-
-
-
-
-
-
+
-    return 0;
-  }
-
+real
-  close(fd);
-size < min
+realsize < size
-  {
-    unlink(file);
@@ -340 +332 @@
-  }
-  
-
+real
-}
-
@@ -371 +363 @@
-  if (unlikely(fd == -1))
-  {
-
-
-
-
+
-    return -1;
-  }
+
-  memsize_t pos = SEEK(fd, 0, SEEK_END);
-  if (unlikely(pos == -1))
-
-
-
-
-
-
+
+
-  return pos;
-}
@@ -391 +377 @@
-bool activate_old_swaps(void)
-{
-  assert(!am_logging());
-
-  DIR *dir = opendir(".");
-  if (unlikely(!dir))
-  {
-perror(
+log_perr(LOG_ERR, 
-    return false;
-  }
@@ -404 +388 @@
-    if (valid_swapfile(d->d_name, &seqno) && !swapfiles[seqno].size)
-    {
-printf("Found old swapfile %d\n
+log_int(LOG_INFO, "Found old swapfile
-      const memsize_t size = filesize(d->d_name); 
-size > min_swapsize && enable_swapfile(d->d_name
+likely(size > min_swapsize && enable_swapfile(d->d_name)
-      {
-        swapfiles[seqno].size = size;
@@ -412 +396 @@
-      else
-      {
-printf("Can't swap to file %d.  Deleting it.\n
+log_int(LOG_NOTICE, "Deleting unusable swap file
-        unlink(d->d_name);
-      }
@@ -428 +412 @@
-{
-  FILE *fp = fopen("/proc/swaps", "r");
-
-
-
-
-
+
-  return fp;
-}
@@ -443 +423 @@
-  if (unlikely(sscanf(localbuf, "Filename Type Size Used %c",&c) < 1))
-  {
-fputs("/proc/swaps is not in the expected format\n", stderr
+log_msg(LOG_ERR, "/proc/swaps is not in the expected format"
-    return false;
-  }
@@ -488 +468 @@
-       * somewhere further down.
-       */
-
-
-
-
-
-
+
+
+
+
+
-    }
-    // /proc/swaps appears to report sizes in 1 KB blocks
@@ -507 +486 @@
-      {
-        // We didn't know about this swapfile yet.  Adopt it.
-
-
-
-
-
-
-
+
-        swapfiles[result->seqno].created = clock;
-      }
@@ -522 +495 @@
-        const memsize_t expected = swapfiles[result->seqno].size,
-                        found = result->size;
-
-
+
-        if (unlikely(swapfiles[result->seqno].observed_in_wild))
-
-
-
-
+
-        else if (unlikely(found > expected))
-
-
-
+
+
+
+
+
-        else if (!quiet && unlikely(found < (expected + PAGE_SIZE)))
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
-      }
-      if (!quiet && unlikely(result->used > result->size))
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
-      swapfiles[result->seqno].size = result->size;
-      swapfiles[result->seqno].used = result->used;
@@ -617 +564 @@
-   * they are still in use or things would get horribly confused.
-   */
-  if (!quiet)
-  {
-    if (am_logging())   syslog(LOG_INFO, "Retiring swapfile %d", file);
-    else                printf("Retiring swapfile %d\n",file);
-  }
-  char namebuf[30];
-  snprintf(namebuf, sizeof(namebuf), "%d", file);
+  if (!quiet) log_str(LOG_NOTICE, "Retiring swapfile", namebuf);
-  if (unlikely(swapoff(namebuf) == -1)) return false;
-  swapfiles[file].size = 0;
@@ -668 +611 @@
-
-
-
-
+
-{
-  /* Round request to page size, then add a bit for swapfile overhead.  Clever
-   * readers will notice that this relies on PAGE_SIZE being a power of two.
-   */
-requested = (requested & ~(PAGE_SIZE-1)
+size = (size & PAGE_MASK
-  const int newswap = find_free(sequence_number);
-  if (swapfiles[newswap].size) return;  // No free slot, sorry!
-
-  // We can allocate another swapfile.  Great.
+  if (!quiet) log_int(LOG_NOTICE, "Allocating swapfile", newswap);
-  char file[30];
-  snprintf(file, sizeof(file), "%d", newswap);
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
-
-  if (swapfiles[newswap].size)

src/swaps.h

@@ -53 +53 @@
-/// Create a new swapfile.  Clobbers localbuf.
-/**
-
-
+
- */
-requested, memsize_t required
+size
-
-/// Free swap space