bionic: make property area expandable

The property area is initially one 4K region, automatically expanding as
needed up to 64 regions.

To avoid duplicating code, __system_property_area_init() now allocates
and initializes the first region (previously it was allocated in init's
init_property_area() and initialized in bionic).  For testing purposes,
__system_property_set_filename() may be used to override the file used
to map in regions.

Signed-off-by: Greg Hackmann <ghackmann@google.com>

(cherry picked from commit d32969701be070c0161c2643ee3c3df16066bbb8)

Change-Id: I038d451fe8849b0c4863663eec6f57f6521bf4a7

libc/bionic/system_properties.c

@@ -55,7 +55,6 @@ struct prop_area {
     unsigned volatile serial;
     unsigned magic;
     unsigned version;
-    unsigned reserved[4];
     unsigned toc[1];
 };
 
@@ -70,10 +69,9 @@ struct prop_info {
 typedef struct prop_info prop_info;
 
 static const char property_service_socket[] = "/dev/socket/" PROP_SERVICE_NAME;
+static char property_filename[PATH_MAX] = PROP_FILENAME;
 
-static unsigned dummy_props = 0;
+prop_area *__system_property_regions__[PA_REGION_COUNT] = { NULL, };
-
-prop_area *__system_property_area__ = (void*) &dummy_props;
 
 static int get_fd_from_env(void)
 {
@@ -86,27 +84,79 @@ static int get_fd_from_env(void)
     return atoi(env);
 }
 
-void __system_property_area_init(void *data)
+static int map_prop_region_rw(size_t region)
 {
-    prop_area *pa = data;
+    prop_area *pa;
+    int fd;
+    size_t offset = region * PA_SIZE;
+
+    if (__system_property_regions__[region]) {
+        return 0;
+    }
+
+    /* dev is a tmpfs that we can use to carve a shared workspace
+     * out of, so let's do that...
+     */
+    fd = open(property_filename, O_RDWR | O_CREAT | O_NOFOLLOW, 0644);
+    if (fd < 0) {
+        if (errno == EACCES) {
+            /* for consistency with the case where the process has already
+             * mapped the page in and segfaults when trying to write to it
+             */
+            abort();
+        }
+        return -1;
+    }
+
+    if (ftruncate(fd, offset + PA_SIZE) < 0)
+        goto out;
+
+    pa = mmap(NULL, PA_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, offset);
+    if(pa == MAP_FAILED)
+        goto out;
+
     memset(pa, 0, PA_SIZE);
     pa->magic = PROP_AREA_MAGIC;
     pa->version = PROP_AREA_VERSION;
 
     /* plug into the lib property services */
-    __system_property_area__ = pa;
+    __system_property_regions__[region] = pa;
+
+    close(fd);
+    return 0;
+
+out:
+    close(fd);
+    return -1;
 }
 
-int __system_properties_init(void)
+int __system_property_set_filename(const char *filename)
+{
+    size_t len = strlen(filename);
+    if (len >= sizeof(property_filename))
+        return -1;
+
+    strcpy(property_filename, filename);
+    return 0;
+}
+
+int __system_property_area_init()
+{
+    return map_prop_region_rw(0);
+}
+
+static int map_prop_region(size_t region)
 {
     bool fromFile = true;
+    bool swapped;
+    size_t offset = region * PA_SIZE;
     int result = -1;
 
-    if(__system_property_area__ != ((void*) &dummy_props)) {
+    if(__system_property_regions__[region]) {
         return 0;
     }
 
-    int fd = open(PROP_FILENAME, O_RDONLY | O_NOFOLLOW);
+    int fd = open(property_filename, O_RDONLY | O_NOFOLLOW);
 
     if ((fd < 0) && (errno == ENOENT)) {
         /*
@@ -133,23 +183,33 @@ int __system_properties_init(void)
 
     if ((fd_stat.st_uid != 0)
             || (fd_stat.st_gid != 0)
-            || ((fd_stat.st_mode & (S_IWGRP | S_IWOTH)) != 0)) {
+            || ((fd_stat.st_mode & (S_IWGRP | S_IWOTH)) != 0)
+            || (fd_stat.st_size < offset + PA_SIZE) ) {
         goto cleanup;
     }
 
-    prop_area *pa = mmap(NULL, fd_stat.st_size, PROT_READ, MAP_SHARED, fd, 0);
+    prop_area *pa = mmap(NULL, PA_SIZE, PROT_READ, MAP_SHARED, fd, offset);
 
     if (pa == MAP_FAILED) {
         goto cleanup;
     }
 
     if((pa->magic != PROP_AREA_MAGIC) || (pa->version != PROP_AREA_VERSION)) {
-        munmap(pa, fd_stat.st_size);
+        munmap(pa, PA_SIZE);
         goto cleanup;
     }
 
-    __system_property_area__ = pa;
     result = 0;
+    swapped = __sync_bool_compare_and_swap(&__system_property_regions__[region],
+            NULL, pa);
+    if (!swapped) {
+        /**
+         * In the event of a race either mapping is equally good, so
+         * the thread that lost can just throw its mapping away and proceed as
+         * normal.
+         */
+        munmap(pa, PA_SIZE);
+    }
 
 cleanup:
     if (fromFile) {
@@ -159,17 +219,31 @@ cleanup:
     return result;
 }
 
+int __system_properties_init()
+{
+    return map_prop_region(0);
+}
+
 int __system_property_foreach(
         void (*propfn)(const prop_info *pi, void *cookie),
         void *cookie)
 {
-    prop_area *pa = __system_property_area__;
+    size_t region;
-    unsigned i;
 
-    for (i = 0; i < pa->count; i++) {
+    for (region = 0; region < PA_REGION_COUNT; region++) {
-        unsigned entry = pa->toc[i];
+        prop_area *pa;
-        prop_info *pi = TOC_TO_INFO(pa, entry);
+        unsigned i;
-        propfn(pi, cookie);
+
+        int err = map_prop_region(region);
+        if (err < 0)
+            break;
+        pa = __system_property_regions__[region];
+
+        for (i = 0; i < pa->count; i++) {
+            unsigned entry = pa->toc[i];
+            prop_info *pi = TOC_TO_INFO(pa, entry);
+            propfn(pi, cookie);
+        }
     }
 
     return 0;
@@ -177,9 +251,15 @@ int __system_property_foreach(
 
 const prop_info *__system_property_find_nth(unsigned n)
 {
-    prop_area *pa = __system_property_area__;
+    size_t region = n / PA_COUNT_MAX;
+    prop_area *pa;
 
-    if(n >= pa->count) {
+    int err = map_prop_region(region);
+    if (err < 0)
+        return NULL;
+    pa = __system_property_regions__[region];
+
+    if((n % PA_COUNT_MAX) >= pa->count) {
         return 0;
     } else {
         return TOC_TO_INFO(pa, pa->toc[n]);
@@ -188,25 +268,36 @@ const prop_info *__system_property_find_nth(unsigned n)
 
 const prop_info *__system_property_find(const char *name)
 {
-    prop_area *pa = __system_property_area__;
-    unsigned count = pa->count;
-    unsigned *toc = pa->toc;
     unsigned len = strlen(name);
-    prop_info *pi;
+    size_t region;
 
     if (len >= PROP_NAME_MAX)
         return 0;
     if (len < 1)
         return 0;
 
-    while(count--) {
+    for (region = 0; region < PA_REGION_COUNT; region++) {
-        unsigned entry = *toc++;
+        prop_area *pa;
-        if(TOC_NAME_LEN(entry) != len) continue;
+        unsigned count;
+        unsigned *toc;
+        prop_info *pi;
 
-        pi = TOC_TO_INFO(pa, entry);
+        int err = map_prop_region(region);
-        if(memcmp(name, pi->name, len)) continue;
+        if (err < 0)
+            return 0;
+        pa = __system_property_regions__[region];
+        count = pa->count;
+        toc = pa->toc;
 
-        return pi;
+        while(count--) {
+            unsigned entry = *toc++;
+            if(TOC_NAME_LEN(entry) != len) continue;
+
+            pi = TOC_TO_INFO(pa, entry);
+            if(memcmp(name, pi->name, len)) continue;
+
+            return pi;
+        }
     }
 
     return 0;
@@ -333,7 +424,7 @@ int __system_property_wait(const prop_info *pi)
 {
     unsigned n;
     if(pi == 0) {
-        prop_area *pa = __system_property_area__;
+        prop_area *pa = __system_property_regions__[0];
         n = pa->serial;
         do {
             __futex_wait(&pa->serial, n, 0);
@@ -349,7 +440,7 @@ int __system_property_wait(const prop_info *pi)
 
 int __system_property_update(prop_info *pi, const char *value, unsigned int len)
 {
-    prop_area *pa = __system_property_area__;
+    prop_area *pa = __system_property_regions__[0];
 
     if (len >= PROP_VALUE_MAX)
         return -1;
@@ -368,12 +459,11 @@ int __system_property_update(prop_info *pi, const char *value, unsigned int len)
 int __system_property_add(const char *name, unsigned int namelen,
             const char *value, unsigned int valuelen)
 {
-    prop_area *pa = __system_property_area__;
+    prop_area *pa;
-    prop_info *pa_info_array = (void*) (((char*) pa) + PA_INFO_START);
+    prop_info *pa_info_array;
     prop_info *pi;
+    size_t region;
 
-    if (pa->count == PA_COUNT_MAX)
-        return -1;
     if (namelen >= PROP_NAME_MAX)
         return -1;
     if (valuelen >= PROP_VALUE_MAX)
@@ -381,13 +471,28 @@ int __system_property_add(const char *name, unsigned int namelen,
     if (namelen < 1)
         return -1;
 
+    for (region = 0; region < PA_REGION_COUNT; region++)
+    {
+        int err = map_prop_region_rw(region);
+        if (err < 0)
+            return -1;
+
+        pa = __system_property_regions__[region];
+
+        if (pa->count < PA_COUNT_MAX)
+            break;
+    }
+
+    if (region == PA_REGION_COUNT)
+        return -1;
+
+    pa_info_array = (void*) (((char*) pa) + PA_INFO_START);
     pi = pa_info_array + pa->count;
     pi->serial = (valuelen << 24);
     memcpy(pi->name, name, namelen + 1);
     memcpy(pi->value, value, valuelen + 1);
 
-    pa->toc[pa->count] =
+    pa->toc[pa->count] = (namelen << 24) | (((unsigned) pi) - ((unsigned) pa));
-        (namelen << 24) | (((unsigned) pi) - ((unsigned) pa));
 
     pa->count++;
     pa->serial++;
@@ -403,7 +508,7 @@ unsigned int __system_property_serial(const prop_info *pi)
 
 unsigned int __system_property_wait_any(unsigned int serial)
 {
-    prop_area *pa = __system_property_area__;
+    prop_area *pa = __system_property_regions__[0];
 
     do {
         __futex_wait(&pa->serial, serial, 0);

libc/include/sys/_system_properties.h

@@ -42,12 +42,13 @@ typedef struct prop_msg prop_msg;
 #define PROP_SERVICE_NAME "property_service"
 #define PROP_FILENAME "/dev/__properties__"
 
-/* (8 header words + 247 toc words) = 1020 bytes */
+/* (4 header words + 28 toc words) = 128 bytes */
-/* 1024 bytes header and toc + 247 prop_infos @ 128 bytes = 32640 bytes */
+/* 128 bytes header and toc + 28 prop_infos @ 128 bytes = 3712 bytes */
 
-#define PA_COUNT_MAX  247
+#define PA_COUNT_MAX    28
-#define PA_INFO_START 1024
+#define PA_REGION_COUNT 128
-#define PA_SIZE       32768
+#define PA_INFO_START   128
+#define PA_SIZE         4096
 
 #define TOC_NAME_LEN(toc)       ((toc) >> 24)
 #define TOC_TO_INFO(area, toc)  ((prop_info*) (((char*) area) + ((toc) & 0xFFFFFF)))
@@ -96,12 +97,18 @@ struct prop_msg
 #define PROP_PATH_LOCAL_OVERRIDE   "/data/local.prop"
 #define PROP_PATH_FACTORY          "/factory/factory.prop"
 
+/*
+** Map the property area from the specified filename.  This
+** method is for testing only.
+*/
+int __system_property_set_filename(const char *filename);
+
 /*
 ** Initialize the area to be used to store properties.  Can
 ** only be done by a single process that has write access to
 ** the property area.
 */
-void __system_property_area_init(void *data);
+int __system_property_area_init();
 
 /* Add a new system property.  Can only be done by a single
 ** process that has write access to the property area, and

tests/property_benchmark.cpp

@@ -15,23 +15,40 @@
  */
 
 #include "benchmark.h"
+#include <unistd.h>
 
 #define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_
 #include <sys/_system_properties.h>
 
 #include <vector>
+#include <string>
 
-extern void *__system_property_area__;
+extern void *__system_property_regions__[PA_REGION_COUNT];
 
 #define TEST_NUM_PROPS \
-    Arg(1)->Arg(4)->Arg(16)->Arg(64)->Arg(128)->Arg(247)
+    Arg(1)->Arg(4)->Arg(16)->Arg(64)->Arg(128)->Arg(256)->Arg(512)->Arg(1024)
 
 struct LocalPropertyTestState {
-    LocalPropertyTestState(int nprops) : nprops(nprops) {
+    LocalPropertyTestState(int nprops) : nprops(nprops), valid(false) {
         static const char prop_name_chars[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ.-_";
-        old_pa = __system_property_area__;
+
-        pa = malloc(PA_SIZE);
+        char dir_template[] = "/data/nativetest/prop-XXXXXX";
-        __system_property_area_init(pa);
+        char *dirname = mkdtemp(dir_template);
+        if (!dirname) {
+            perror("making temp file for test state failed (is /data/nativetest writable?)");
+            return;
+        }
+
+        for (size_t i = 0; i < PA_REGION_COUNT; i++) {
+            old_pa[i] = __system_property_regions__[i];
+            __system_property_regions__[i] = NULL;
+        }
+
+        pa_dirname = dirname;
+        pa_filename = pa_dirname + "/__properties__";
+
+        __system_property_set_filename(pa_filename.c_str());
+        __system_property_area_init();
 
         names = new char* [nprops];
         name_lens = new int[nprops];
@@ -54,10 +71,22 @@ struct LocalPropertyTestState {
             }
             __system_property_add(names[i], name_lens[i], values[i], value_lens[i]);
         }
+
+        valid = true;
     }
 
     ~LocalPropertyTestState() {
-        __system_property_area__ = old_pa;
+        if (!valid)
+            return;
+
+        for (size_t i = 0; i < PA_REGION_COUNT; i++) {
+            __system_property_regions__[i] = old_pa[i];
+        }
+
+        __system_property_set_filename(PROP_FILENAME);
+        unlink(pa_filename.c_str());
+        rmdir(pa_dirname.c_str());
+
         for (int i = 0; i < nprops; i++) {
             delete names[i];
             delete values[i];
@@ -66,7 +95,6 @@ struct LocalPropertyTestState {
         delete[] name_lens;
         delete[] values;
         delete[] value_lens;
-        free(pa);
     }
 public:
     const int nprops;
@@ -74,10 +102,12 @@ public:
     int *name_lens;
     char **values;
     int *value_lens;
+    bool valid;
 
 private:
-    void *pa;
+    std::string pa_dirname;
-    void *old_pa;
+    std::string pa_filename;
+    void *old_pa[PA_REGION_COUNT];
 };
 
 static void BM_property_get(int iters, int nprops)
@@ -87,6 +117,9 @@ static void BM_property_get(int iters, int nprops)
     LocalPropertyTestState pa(nprops);
     char value[PROP_VALUE_MAX];
 
+    if (!pa.valid)
+        return;
+
     srandom(iters * nprops);
 
     StartBenchmarkTiming();
@@ -104,6 +137,9 @@ static void BM_property_find(int iters, int nprops)
 
     LocalPropertyTestState pa(nprops);
 
+    if (!pa.valid)
+        return;
+
     srandom(iters * nprops);
 
     StartBenchmarkTiming();

tests/system_properties_test.cpp

@@ -16,32 +16,61 @@
 
 #include <gtest/gtest.h>
 #include <sys/wait.h>
+#include <unistd.h>
+#include <string>
 
 #if __BIONIC__
 
 #define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_
 #include <sys/_system_properties.h>
 
-extern void *__system_property_area__;
+extern void *__system_property_regions__[PA_REGION_COUNT];
 
 struct LocalPropertyTestState {
-    LocalPropertyTestState() {
+    LocalPropertyTestState() : valid(false) {
-        old_pa = __system_property_area__;
+        char dir_template[] = "/data/nativetest/prop-XXXXXX";
-        pa = malloc(PA_SIZE);
+        char *dirname = mkdtemp(dir_template);
-        __system_property_area_init(pa);
+        if (!dirname) {
+            perror("making temp file for test state failed (is /data/nativetest writable?)");
+            return;
+        }
+
+        for (size_t i = 0; i < PA_REGION_COUNT; i++) {
+            old_pa[i] = __system_property_regions__[i];
+            __system_property_regions__[i] = NULL;
+        }
+
+        pa_dirname = dirname;
+        pa_filename = pa_dirname + "/__properties__";
+
+        __system_property_set_filename(pa_filename.c_str());
+        __system_property_area_init();
+        valid = true;
     }
 
     ~LocalPropertyTestState() {
-        __system_property_area__ = old_pa;
+        if (!valid)
-        free(pa);
+            return;
+
+        for (size_t i = 0; i < PA_REGION_COUNT; i++) {
+            __system_property_regions__[i] = old_pa[i];
+        }
+
+        __system_property_set_filename(PROP_FILENAME);
+        unlink(pa_filename.c_str());
+        rmdir(pa_dirname.c_str());
     }
+public:
+    bool valid;
 private:
-    void *pa;
+    std::string pa_dirname;
-    void *old_pa;
+    std::string pa_filename;
+    void *old_pa[PA_REGION_COUNT];
 };
 
 TEST(properties, add) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
 
     char propvalue[PROP_VALUE_MAX];
 
@@ -61,6 +90,7 @@ TEST(properties, add) {
 
 TEST(properties, update) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
 
     char propvalue[PROP_VALUE_MAX];
     prop_info *pi;
@@ -91,27 +121,34 @@ TEST(properties, update) {
     ASSERT_STREQ(propvalue, "value6");
 }
 
-// 247 = max # of properties supported by current implementation
+TEST(properties, fill) {
-// (this should never go down)
-TEST(properties, fill_247) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
     char prop_name[PROP_NAME_MAX];
     char prop_value[PROP_VALUE_MAX];
     char prop_value_ret[PROP_VALUE_MAX];
+    int count = 0;
     int ret;
 
-    for (int i = 0; i < 247; i++) {
+    while (true) {
-        ret = snprintf(prop_name, PROP_NAME_MAX - 1, "property_%d", i);
+        ret = snprintf(prop_name, PROP_NAME_MAX - 1, "property_%d", count);
         memset(prop_name + ret, 'a', PROP_NAME_MAX - 1 - ret);
-        ret = snprintf(prop_value, PROP_VALUE_MAX - 1, "value_%d", i);
+        ret = snprintf(prop_value, PROP_VALUE_MAX - 1, "value_%d", count);
         memset(prop_value + ret, 'b', PROP_VALUE_MAX - 1 - ret);
         prop_name[PROP_NAME_MAX - 1] = 0;
         prop_value[PROP_VALUE_MAX - 1] = 0;
 
-        ASSERT_EQ(0, __system_property_add(prop_name, PROP_NAME_MAX - 1, prop_value, PROP_VALUE_MAX - 1));
+        ret = __system_property_add(prop_name, PROP_NAME_MAX - 1, prop_value, PROP_VALUE_MAX - 1);
+        if (ret < 0)
+            break;
+
+        count++;
     }
 
-    for (int i = 0; i < 247; i++) {
+    // For historical reasons at least 247 properties must be supported
+    ASSERT_GE(count, 247);
+
+    for (int i = 0; i < count; i++) {
         ret = snprintf(prop_name, PROP_NAME_MAX - 1, "property_%d", i);
         memset(prop_name + ret, 'a', PROP_NAME_MAX - 1 - ret);
         ret = snprintf(prop_value, PROP_VALUE_MAX - 1, "value_%d", i);
@@ -134,6 +171,7 @@ static void foreach_test_callback(const prop_info *pi, void* cookie) {
 
 TEST(properties, foreach) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
     size_t count = 0;
 
     ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6));
@@ -146,6 +184,7 @@ TEST(properties, foreach) {
 
 TEST(properties, find_nth) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
 
     ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6));
     ASSERT_EQ(0, __system_property_add("other_property", 14, "value2", 6));
@@ -165,6 +204,7 @@ TEST(properties, find_nth) {
 
 TEST(properties, errors) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
     char prop_value[PROP_NAME_MAX];
 
     ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6));
@@ -181,6 +221,7 @@ TEST(properties, errors) {
 
 TEST(properties, serial) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
     const prop_info *pi;
     unsigned int serial;
 
@@ -206,6 +247,7 @@ static void *PropertyWaitHelperFn(void *arg)
 
 TEST(properties, wait) {
     LocalPropertyTestState pa;
+    ASSERT_TRUE(pa.valid);
     unsigned int serial;
     prop_info *pi;
     pthread_t t;