Hello,
In my opinion early init is too late as we need /tmp sooner - for example to be
able
to mount overlayfs (even init & procd needs writeable /tmp!). Maybe we can
temporaily
mount "something" (RAMFS/TMPFS depending on kernel configuration) on /tmp and
then
remount ZRAM but I do not think it will be better as we will have to copy stuff
over...
I can provide patch using correct coding style if you need.
Please find answers to your comments below.
Regards,
Thomas
W dniu 22.10.2014 o 12:39, John Crispin pisze:
> Hi,
>
> a few comments inline. i am not sure if we want this inside procd
> directly. maybe we should put this into fstools and then call that code
> from early init. i need to think about that before i can make a decision.
>
> John
>
>
>
> On 22/10/2014 10:30, Tomasz Wasiak wrote:
>> Hello,
>>
>> Here you are (of course it is just 1/3 of the original patch).
>> ---
>> diff --git a/initd/early.c b/initd/early.c
>> index a9f6afb..b4a375f 100644
>> --- a/initd/early.c
>> +++ b/initd/early.c
>> @@ -12,34 +12,130 @@
>> * GNU General Public License for more details.
>> */
>>
>> -#include <sys/mount.h>
>> -#include <sys/types.h>
>> -#include <sys/stat.h>
>> -
>> -#include <stdio.h>
>> +#include <errno.h>
>> #include <fcntl.h>
>> -#include <unistd.h>
>> +#include <stdio.h>
>> #include <stdlib.h>
>> +#include <string.h>
>> +#include <strings.h>
>> +#include <unistd.h>
>> +#include <sys/mount.h>
>> +#include <sys/stat.h>
>> +#include <sys/types.h>
>> +#include <sys/wait.h>
>>
>> #include "../log.h"
>> #include "init.h"
>>
>> +static long
>> +check_ramsize(void)
>> +{
>> + FILE *fp;
>> + char line[256];
>> + char *key;
>> + long val = 0;
>> +
>> + fp = fopen("/proc/meminfo", "r");
>> + if(fp == NULL)
>> + {
>
> please use this coding style ->
>
> if (fp == NULL) {
>
> }
>
>
>> + ERROR("Can't open /proc/meminfo: %s\n", strerror(errno));
>> + return errno;
>> + }
>> +
>> + while(fgets(line, sizeof(line), fp))
>> + {
>
> and here aswell
>
> while () {
>
> }
>
>> + key = strtok(line, ":");
>> + val = atol(strtok(NULL, " kB\n"));
>
> what happens if strtok return NULL ?
SIGSEGV :(
But that means that /proc/meminfo does not contain information about total
memory.
Safer solution would be:
char *key, *val;
(...)
key = strtok(line, ":");
val = strtok(NULL, " kB\n");
(...)
break;
fclose(fp);
if(val != NULL)
return(atol(val));
else
return 0;
fi
}
>
>> +
>> + if (!key || !val)
>> + continue;
>> +
>> + if (!strcasecmp(key, "MemTotal"))
>> + break;
>> + }
>> +
>> + fclose(fp);
>> +
>> + return val;
>> +}
>> +
>> +static int
>> +mount_zram_on_tmp(void)
>> +{
>> + FILE *fp;
>> + long zramsize = (check_ramsize() / 2);
>
> where does the /2 come from ? is this just "a good value to use" or is
> there a technical reason that is must always be /2 and not /X
/2 is here because I would like to limit memory used by /tmp filesystem to half
of RAM.
Now it is not limited so it can eat all memory! IMHO even if a device has only
16 MB
of RAM 8 MB would be sufficient for /tmp (even when it will be pure
RAMFS/TMPFS).
>
>> + pid_t pid;
>> +
>> + if(!zramsize)
>> + {
>> + ERROR("Can't read size of RAM. Assuming 16 MB.\n");
>> + zramsize = 8192;
>
> dangerous, if the memory was not detected properly we have mayor issues
> and should not do any magic with the memory. in this case we should just
> continue without zram
Of course but that way we are not able to limit /tmp memory usage. IMHO
assumption that
device has at least 16 MB of RAM is not harmful as I do not think one can run
newer builds
(based on 3.x kernels and modern toolchains) on any device equipped with 8 (or
even less)
MB of RAM.
>
>> + }
>> +
>> + fp = fopen("/sys/block/zram0/disksize", "r+");
>> + if(fp == NULL)
>> + {
>> + ERROR("Can't open /sys/block/zram0/disksize: %s\n",
>> strerror(errno));
>> + return errno;
>> + }
>> +
>> + fprintf(fp, "%ld", (zramsize * 1024));
>> +
>> + fclose(fp);
>> +
>> + pid = fork();
>> +
>> + if (!pid)
>> + {
>> + char *mkfs[] = { "/sbin/mke2fs", "-b", "4096", "-F", "-L",
>> "TEMP", "-m", "0", "/dev/zram0", NULL };
>> + int fd = open("/dev/null", O_RDWR);
>> +
>> + if (fd > -1) {
>> + dup2(fd, STDIN_FILENO);
>> + dup2(fd, STDOUT_FILENO);
>> + dup2(fd, STDERR_FILENO);
>> + if (fd > STDERR_FILENO)
>> + close(fd);
>> + }
>> +
>> + execvp(mkfs[0], mkfs);
>> + ERROR("Can't exec /sbin/mke2fs\n");
>> + exit(-1);
>> + }
>> +
>> + if (pid <= 0)
>> + {
>> + ERROR("Can't exec /sbin/mke2fs\n");
>> + return -1;
>> + } else {
>> + waitpid(pid, NULL, 0);
>> + }
>> +
>> + if(mount("/dev/zram0", "/tmp", "ext2", MS_NOSUID | MS_NODEV |
>> MS_NOATIME, "check=none,errors=continue,noquota") < 0)
>> + {
>> + ERROR("Can't mount /dev/zram0 on /tmp: %s\n", strerror(errno));
>> + return errno;
>> + }
>> +
>> + LOG("Using up to %ld kB of RAM as ZRAM storage on /mnt\n", zramsize);
>> + return 0;
>> +}
>> +
>> static void
>> -early_mounts(void)
>> +mount_tmpfs_on_tmp(void)
>> {
>> - mount("proc", "/proc", "proc", MS_NOATIME, 0);
>> - mount("sysfs", "/sys", "sysfs", MS_NOATIME, 0);
>> + char line[256];
>> + long tmpfssize = (check_ramsize() / 2);
>
> same here
>
>>
>> - mount("tmpfs", "/tmp", "tmpfs", MS_NOSUID | MS_NODEV | MS_NOATIME,
>> NULL);
>> - mkdir("/tmp/run", 0777);
>> - mkdir("/tmp/lock", 0777);
>> - mkdir("/tmp/state", 0777);
>> - symlink("/tmp", "/var");
>> + if(!tmpfssize)
>> + {
>> + ERROR("Can't read size of RAM. Assuming 16 MB.\n");
>> + tmpfssize = 8192;
>
> same here
>
>> + }
>>
>> - mount("tmpfs", "/dev", "tmpfs", MS_NOATIME, "mode=0755,size=512K");
>> - mkdir("/dev/shm", 0755);
>> - mkdir("/dev/pts", 0755);
>> - mount("devpts", "/dev/pts", "devpts", MS_NOATIME, "mode=600");
>> + snprintf(line, 256, "size=%ldk", tmpfssize);
>> + mount("tmpfs", "/tmp", "tmpfs", MS_NOSUID | MS_NODEV | MS_NOATIME,
>> line);
>> + LOG("Using up to %ld kB of RAM as TMPFS storage on /tmp\n", tmpfssize);
>> }
>>
>> static void
>> @@ -50,6 +146,25 @@ early_dev(void)
>> }
>>
>> static void
>> +early_mounts(void)
>> +{
>> + mount("proc", "/proc", "proc", MS_NOATIME, 0);
>> + mount("sysfs", "/sys", "sysfs", MS_NOATIME, 0);
>> + mount("tmpfs", "/dev", "tmpfs", MS_NOATIME, "mode=0755,size=512K");
>> + mkdir("/dev/shm", 0755);
>> + mkdir("/dev/pts", 0755);
>> + mount("devpts", "/dev/pts", "devpts", MS_NOATIME, "mode=600");
>> + early_dev();
>> +
>> + if(mount_zram_on_tmp() !=0)
>> + mount_tmpfs_on_tmp();
>> + mkdir("/tmp/run", 0777);
>> + mkdir("/tmp/lock", 0777);
>> + mkdir("/tmp/state", 0777);
>> + symlink("/tmp", "/var");
>> +}
>> +
>> +static void
>> early_console(const char *dev)
>> {
>> struct stat s;
>> @@ -89,7 +204,6 @@ early(void)
>> return;
>>
>> early_mounts();
>> - early_dev();
>> early_env();
>> early_console("/dev/console");
>>
>>
>>
>> W dniu 22.10.2014 o 09:38, John Crispin pisze:
>>> Hi,
>>>
>>> please send patches directly against the procd git tree.
>>>
>>> John
>>>
>>> On 22/10/2014 09:20, Tomasz Wasiak wrote:
>>>> Devices with less memory are still common so why limit ZRAM usage
>>>> just to swap when it could be very useful as for /tmp storage.
>>>>
>>>> This patch changes 3 things: - sets default number of ZRAM devices
>>>> to 2 (1st for /tmp, 2nd for swap) - adds ZRAM (default) support to
>>>> procd's init (TMPFS is used when ZRAM is not available - changes
>>>> zram-swap so it will use /dev/zram1 for 1st CPU, /dev/zram2 for
>>>> 2nd and so on as /dev/zram0 is in use for /tmp.
>>>>
>>>> Signed-off-by: Tomasz Wasiak <tjwas...@gmail.com> ---
>>>> package/system/procd/patches/procd-support_for_tmp_on_zram.patch
>>>> | 185 ++++++++++ package/system/zram-swap/files/zram.init
>>>> | 15
>>>> target/linux/generic/patches-3.10/998_zram_make_2_devices_by_default.patch
>>>> | 11 3 files changed, 203 insertions(+), 8 deletions(-)
>>>>
>>>> ---
>>>> a/package/system/procd/patches/procd-support_for_tmo_on_zram.patch
>>>> +++
>>>> b/package/system/procd/patches/procd-support_for_tmp_on_zram.patch
>>>> @@ -0,0 +1,185 @@ +--- a/initd/early.c ++++ b/initd/early.c +@@
>>>> -12,34 +12,130 @@ + * GNU General Public License for more
>>>> details. + */ + +-#include <sys/mount.h> +-#include <sys/types.h>
>>>> +-#include <sys/stat.h> +- +-#include <stdio.h> ++#include
>>>> <errno.h> + #include <fcntl.h> +-#include <unistd.h> ++#include
>>>> <stdio.h> + #include <stdlib.h> ++#include <string.h> ++#include
>>>> <strings.h> ++#include <unistd.h> ++#include <sys/mount.h>
>>>> ++#include <sys/stat.h> ++#include <sys/types.h> ++#include
>>>> <sys/wait.h> + + #include "../log.h" + #include "init.h" + ++static
>>>> long ++check_ramsize(void) ++{ ++ FILE *fp; ++ char line[256]; ++
>>>> char *key; ++ long val = 0; ++ ++ fp = fopen("/proc/meminfo",
>>>> "r"); ++ if(fp == NULL) ++ { ++ ERROR("Can't open
>>>> /proc/meminfo:
>>>> %s\n", strerror(errno)); ++ return errno; ++ } ++ ++
>>>> while(fgets(line, sizeof(line), fp)) ++ { ++ key =
>>>> strtok(line,
>>>> ":"); ++ val = atol(strtok(NULL, " kB\n")); ++ ++
>>>> if (!key ||
>>>> !val) ++ continue; ++ ++ if (!strcasecmp(key,
>>>> "MemTotal")) ++
>>>> break; ++ } ++ ++ fclose(fp); ++ ++ return val; ++} ++ ++static
>>>> int ++mount_zram_on_tmp(void) ++{ ++ FILE *fp; ++ long zramsize =
>>>> (check_ramsize() / 2); ++ pid_t pid; ++ ++ if(!zramsize) ++
>>>> { ++
>>>> ERROR("Can't read size of RAM. Assuming 16 MB.\n"); ++
>>>> zramsize =
>>>> 8192; ++ } ++ ++ fp = fopen("/sys/block/zram0/disksize", "r+"); ++
>>>> if(fp == NULL) ++ { ++ ERROR("Can't open
>>>> /sys/block/zram0/disksize: %s\n", strerror(errno)); ++ return
>>>> errno; ++ } ++ ++ fprintf(fp, "%ld", (zramsize * 1024)); ++ ++
>>>> fclose(fp); ++ ++ pid = fork(); ++ ++ if (!pid) ++ { ++
>>>> char
>>>> *mkfs[] = { "/sbin/mke2fs", "-b", "4096", "-F", "-L", "TEMP", "-m",
>>>> "0", "/dev/zram0", NULL }; ++ int fd = open("/dev/null",
>>>> O_RDWR);
>>>> ++ ++ if (fd > -1) { ++ dup2(fd,
>>>> STDIN_FILENO); ++ dup2(fd,
>>>> STDOUT_FILENO); ++ dup2(fd, STDERR_FILENO); ++
>>>> if (fd >
>>>> STDERR_FILENO) ++ close(fd); ++ } ++ ++
>>>> execvp(mkfs[0],
>>>> mkfs); ++ ERROR("Can't exec /sbin/mke2fs\n"); ++
>>>> exit(-1); ++ }
>>>> ++ ++ if (pid <= 0) ++ { ++ ERROR("Can't exec
>>>> /sbin/mke2fs\n"); ++
>>>> return -1; ++ } else { ++ waitpid(pid, NULL, 0); ++
>>>> } ++ ++
>>>> if(mount("/dev/zram0", "/tmp", "ext2", MS_NOSUID | MS_NODEV |
>>>> MS_NOATIME, "check=none,errors=continue,noquota") < 0) ++ { ++
>>>> ERROR("Can't mount /dev/zram0 on /tmp: %s\n", strerror(errno)); ++
>>>> return errno; ++ } ++ ++ LOG("Using up to %ld kB of RAM as ZRAM
>>>> storage on /mnt\n", zramsize); ++ return 0; ++} ++ + static void
>>>> +-early_mounts(void) ++mount_tmpfs_on_tmp(void) + { +-
>>>> mount("proc", "/proc", "proc", MS_NOATIME, 0); +- mount("sysfs",
>>>> "/sys", "sysfs", MS_NOATIME, 0); ++ char line[256]; ++ long
>>>> tmpfssize = (check_ramsize() / 2); + +- mount("tmpfs", "/tmp",
>>>> "tmpfs", MS_NOSUID | MS_NODEV | MS_NOATIME, NULL); +-
>>>> mkdir("/tmp/run", 0777); +- mkdir("/tmp/lock", 0777); +-
>>>> mkdir("/tmp/state", 0777); +- symlink("/tmp", "/var"); ++
>>>> if(!tmpfssize) ++ { ++ ERROR("Can't read size of RAM. Assuming
>>>> 16
>>>> MB.\n"); ++ tmpfssize = 8192; ++ } + +- mount("tmpfs",
>>>> "/dev",
>>>> "tmpfs", MS_NOATIME, "mode=0755,size=512K"); +- mkdir("/dev/shm",
>>>> 0755); +- mkdir("/dev/pts", 0755); +- mount("devpts", "/dev/pts",
>>>> "devpts", MS_NOATIME, "mode=600"); ++ snprintf(line, 256,
>>>> "size=%ldk", tmpfssize); ++ mount("tmpfs", "/tmp", "tmpfs",
>>>> MS_NOSUID | MS_NODEV | MS_NOATIME, line); ++ LOG("Using up to %ld
>>>> kB of RAM as TMPFS storage on /tmp\n", tmpfssize); + } + + static
>>>> void +@@ -50,6 +146,25 @@ early_dev(void) + } + + static void
>>>> ++early_mounts(void) ++{ ++ mount("proc", "/proc", "proc",
>>>> MS_NOATIME, 0); ++ mount("sysfs", "/sys", "sysfs", MS_NOATIME, 0);
>>>> ++ mount("tmpfs", "/dev", "tmpfs", MS_NOATIME,
>>>> "mode=0755,size=512K"); ++ mkdir("/dev/shm", 0755); ++
>>>> mkdir("/dev/pts", 0755); ++ mount("devpts", "/dev/pts", "devpts",
>>>> MS_NOATIME, "mode=600"); ++ early_dev(); ++ ++
>>>> if(mount_zram_on_tmp() !=0) ++ mount_tmpfs_on_tmp(); ++
>>>> mkdir("/tmp/run", 0777); ++ mkdir("/tmp/lock", 0777); ++
>>>> mkdir("/tmp/state", 0777); ++ symlink("/tmp", "/var"); ++} ++
>>>> ++static void + early_console(const char *dev) + { + struct stat
>>>> s; +@@ -87,7 +202,6 @@ early(void) + return; + +
>>>> early_mounts(); +- early_dev(); + early_env(); +
>>>> early_console("/dev/console"); + ---
>>>> a/package/system/zram-swap/files/zram.init +++
>>>> b/package/system/zram-swap/files/zram.init @@ -15,10 +15,10 @@
>>>> zram_size() # in megabytes local ram_size="$( ram_size )"
>>>>
>>>> if [ -z "$zram_size" ]; then - # e.g. 6mb for 16mb-routers or
>>>> 61mb
>>>> for 128mb-routers - echo $(( $ram_size / 2048 )) +
>>>> # e.g. 3200kb
>>>> for 16mb-routers or 25600kb for 128mb-routers + echo $((
>>>> $ram_size
>>>> * 2 / 10 )) else - echo "$zram_size" + echo $((
>>>> $zram_size * 1024
>>>> )) fi }
>>>>
>>>> @@ -54,9 +54,9 @@ zram_applicable()
>>>>
>>>> zram_dev() { - local core="$1" + local core=$(( $1 + 1 ))
>>>>
>>>> - echo "/dev/zram${core:-0}" + echo "/dev/zram${core:-1}" }
>>>>
>>>> zram_reset() @@ -95,10 +95,10 @@ start() zram_dev="$( zram_dev
>>>> "$core" )" zram_applicable "$zram_dev" || return 1
>>>>
>>>> - logger -s -t zram_start -p daemon.debug "activating '$zram_dev'
>>>> for swapping ($zram_size MegaBytes)" + logger -s -t zram_start
>>>> -p
>>>> daemon.debug "activating '$zram_dev' for swapping ($zram_size
>>>> KiloBytes)"
>>>>
>>>> zram_reset "$zram_dev" "enforcing defaults" - echo $((
>>>> $zram_size
>>>> * 1024 * 1024 )) >"/sys/block/$( basename $zram_dev )/disksize" +
>>>> echo $(( $zram_size * 1024 )) >"/sys/block/$( basename $zram_dev
>>>> )/disksize" mkswap "$zram_dev" swapon "$zram_dev" } done @@ -120,4
>>>> +120,3 @@ stop() zram_reset "$zram_dev" "claiming memory back" }
>>>> done } - ---
>>>> a/target/linux/generic/patches-3.10/998_zram_make_2_devices_by_default.patch
>>>>
>>>>
>>> +++
>>> b/target/linux/generic/patches-3.10/998_zram_make_2_devices_by_default.patch
>>>> @@ -0,0 +1,11 @@ +--- a/drivers/staging/zram/zram_drv.c ++++
>>>> b/drivers/staging/zram/zram_drv.c +@@ -40,7 +40,7 @@ static int
>>>> zram_major; + struct zram *zram_devices; + + /* Module params
>>>> (documentation at end) */ +-static unsigned int num_devices = 1;
>>>> ++static unsigned int num_devices = 2; + + static void
>>>> zram_stat64_add(struct zram *zram, u64 *v, u64 inc) + {
>>>> _______________________________________________ openwrt-devel
>>>> mailing list openwrt-devel@lists.openwrt.org
>>>> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>>>>
>>> _______________________________________________
>>> openwrt-devel mailing list
>>> openwrt-devel@lists.openwrt.org
>>> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>>>
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
