dev: shrink and cache-align struct mog_dev

The semaphore inside the dev struct will be accessed by
multiple threads frequently, so keep it cache-aligned.

To reduce memory usage for large-numbered devices, avoid
storing the prefix on output and instead just rely on
the printf-family of routines to generate stringified output
in uncommon code paths.

4 files changed, 46 insertions, 21 deletions

diff --git a/cmogstored.h b/cmogstored.h
index 9abe175..18131b7 100644
--- a/cmogstored.h
+++ b/cmogstored.h
@@ -104,7 +104,6 @@ struct mog_dev {
         unsigned max_active;
         unsigned cur_max;
         sem_t sem;
-        char prefix[FLEXIBLE_ARRAY_MEMBER];
 };
 
 struct mog_rbuf {
diff --git a/dev.c b/dev.c
index 551b15c..f6bd43d 100644
--- a/dev.c
+++ b/dev.c
@@ -4,25 +4,31 @@
  */
 #include "cmogstored.h"
 
+static int stat_prefix(struct mog_svc *svc, struct stat *sb, uint32_t mog_devid)
+{
+        char prefix[sizeof("/dev" MOG_STR(MOG_DEVID_MAX) "/")];
+        int rc;
+
+        assert(mog_devid <= MOG_DEVID_MAX && "mog_devid not filtered");
+        rc = snprintf(prefix, sizeof(prefix), "/dev%u/", mog_devid);
+        assert(rc > 0 && rc < (int)sizeof(prefix) && "we suck at snprintf");
+
+        return mog_stat(svc, prefix, sb);
+}
+
 static struct mog_dev *mog_dev_new(struct mog_svc *svc, uint32_t mog_devid)
 {
         struct mog_dev *dev;
         struct stat sb;
-        char *devprefix = xasprintf("/dev%u/", mog_devid);
-        size_t len = strlen(devprefix);
 
-        if (mog_stat(svc, devprefix, &sb) < 0) {
-                PRESERVE_ERRNO( free(devprefix) );
+        /* we have no space on stack */
+        if (mog_devid > MOG_DEVID_MAX)
                 return NULL;
-        }
 
-        dev = xmalloc(sizeof(struct mog_dev) + len);
-
-        assert(devprefix[len - 1] == '/' && "not trailing slash");
-        devprefix[len - 1] = '\0';
-        memcpy(dev->prefix, devprefix, len);
-        free(devprefix);
+        if (stat_prefix(svc, &sb, mog_devid) < 0)
+                return NULL;
 
+        dev = mog_cachealign(sizeof(struct mog_dev));
         dev->cur_max = dev->max_active = 1;
         CHECK(int, 0, sem_init(&dev->sem, 0, 1));
 
@@ -53,7 +59,7 @@ mog_dev_for(struct mog_svc *svc, uint32_t mog_devid, bool update)
                  * Possible FS/device error, it could come back, so do
                  * not remove here.
                  */
-                if (mog_stat(svc, ret->prefix, &sb) < 0)
+                if (stat_prefix(svc, &sb, ret->devid) < 0)
                         goto out;
 
                 /* st_dev may change due to remount, update if needed */
@@ -119,7 +125,7 @@ const struct mog_dev *dev, struct mog_svc *svc, int fd, struct statvfs *v)
                 static const char usage_fmt[] =
                         "available: %llu\n"
                         "device: %s\n"
-                        "disk: %s%s\n"
+                        "disk: %s/dev%u\n"
                         "time: %lld\n"
                         "total: %llu\n"
                         "use: %u%%\n"
@@ -128,18 +134,18 @@ const struct mog_dev *dev, struct mog_svc *svc, int fd, struct statvfs *v)
                 errno = 0;
                 rc = dprintf(fd, usage_fmt,
                              available, me->me_devname, svc->docroot,
-                             dev->prefix, now, total, use, used);
+                             (unsigned)dev->devid, now, total, use, used);
 
                 PRESERVE_ERRNO( mog_mnt_release(me) );
                 if (rc < 0 || errno == ENOSPC) {
                         PRESERVE_ERRNO(do {
-                                syslog(LOG_ERR, "dprintf(%s%s/usage): %m",
-                                       svc->docroot, dev->prefix);
+                                syslog(LOG_ERR, "dprintf(%s/dev%u/usage): %m",
+                                       svc->docroot, (unsigned)dev->devid);
                         } while (0));
                 }
         } else {
-                syslog(LOG_ERR, "mount entry not found for %s%s",
-                       svc->docroot, dev->prefix);
+                syslog(LOG_ERR, "mount entry not found for %s/dev%u",
+                       svc->docroot, (unsigned)dev->devid);
                 errno = ENODEV;
         }
 
@@ -156,7 +162,7 @@ int mog_dev_mkusage(const struct mog_dev *dev, struct mog_svc *svc)
         if (!svc->mgmt_mfd)
                 return 0;
 
-        usage_path = xasprintf("%s/usage", dev->prefix);
+        usage_path = xasprintf("/dev%u/usage", (unsigned)dev->devid);
         tmp_path = xasprintf("%s.%x", usage_path, (unsigned)getpid());
 
         if (mog_unlink(svc, tmp_path) < 0 && errno != ENOENT) goto out;
diff --git a/path_parser.rl b/path_parser.rl
index fc77c98..e403607 100644
--- a/path_parser.rl
+++ b/path_parser.rl
@@ -11,7 +11,7 @@
 
         devid = "dev"
                 (digit+) $ {
-                        /* no overflow checking here, no need */
+                        /* no overflow checking here, we do it in mog_dev_new */
                         *mog_devid *= 10;
                         *mog_devid += fc - '0';
                 }
diff --git a/test/mgmt.rb b/test/mgmt.rb
index 373cd69..c03ec7a 100644
--- a/test/mgmt.rb
+++ b/test/mgmt.rb
@@ -288,6 +288,26 @@ class TestMgmt < Test::Unit::TestCase
     assert_match(%r{ERROR: unknown command}, @client.gets)
   end
 
+  def test_giant_devid_skip
+    max = 16777215 # devid is MEDIUMINT in DB
+    Dir.mkdir("#@tmpdir/dev#{max}")
+    Dir.mkdir("#@tmpdir/dev#{max + 1}")
+    @client.write "watch\n"
+    lines = []
+
+    2.times do # 2 times in case we're slow
+      begin
+        line = @client.gets
+        lines << line
+      end until line == ".\n"
+    end
+
+    assert lines.grep(/\b#{max}\b/)[0]
+    assert_nil lines.grep(/\b#{max + 1}\b/)[0]
+    assert File.exist?("#@tmpdir/dev#{max}/usage")
+    assert ! File.exist?("#@tmpdir/dev#{max + 1}/usage")
+  end
+
   def test_iostat_watch
     Dir.mkdir("#@tmpdir/dev666")
     @client.write "watch\n"