On 07/03/2016 01:24 PM, John Crispin wrote: > Hi, > > few comments > > 1) the patch needs to be split into 2, ext2 change/lzo change > 2) send them inline please and not as attachement > 3) the V3 needs to be in the subject prefix [PATCH V3] procd: ... > 4) the Signed-off-by: line is missing > > John
Hi John, 1) no can do. You are going to merge it into your project git page anyway, unless openwrt locked you out of your own project you are a maintainer of on git. In which case reply with the rebuke to .... 2) ok I fired up thunderbird, google mail destroys patches in plain text mode 3) done 4) done Don't blame me for the long email. I thought I was already done earlier but ..... I have tested this with lede trunk with v2 of the patch. V3 is the same just with a #ifdef, lz4 (faster less compression). It definitely works. It has 4mb flash. 140k of text files generated in my setup and placed in /tmp turned into 7.1k - neat! - however tmpfs has garbage collection zram doesn't exit unless you create a cron job that will dd if=/dev/zero bs=4096 of=/tmp/zero.bin count=sizeofreespace; rm /tmp/zero.bin. Deleting a file from /tmp doesn't allow memory reclamation like with tmpfs. Under normal use it will take a long time for this to really fill up, especially if ext2 is over-writing the same files & inodes. Zram module estimates there is 28meg of /tmp storage which is more than you'd get with tmpfs. My tp-link has 32mb ram 28 usable. Secure deletion on ext4 was never popular: https://lwn.net/Articles/462437/ Also of NOTE is tmpfs is STILL getting mounted. People with this feature on could in theory disable tmpfs. John you might want to look into disabling the tmpfs mount of ZRAM_TMPFS mounted instead. John: Constraints for the Makefile for procd to go with this I didn't submit, because I'm not sure config stanzas like package stanzas can have DEPENDS. Maybe you have to make it a fake package or something that really just passes on the defines to gcc so you can set up package dependencies. ===================================== 1) Thou shall turn on CONFIG_BUSYBOX_CONFIG_MKFS_EXT2=y Justification: ~15-20k binary vs 350k (with stripping) for mkfs.ext2 and if you want the proper mkfs.ext4 etc you can install it in your overlay - extroot. Busybox mkfs.ext2 and e2fsprogs are configured not to collide with each-other. This patch makes the need for e2fsprogs libext2 redundant. Not having this (like we do right now) will flood the bug reports with idiots (like me) wondering why they bricked their router when it can't mount /tmp. 2) Debatable: Thou shall disable zram-swap, zram-swap shall block this PROC_ZRAM_TMPFS option. Justification: Right now they both want to use /dev/zram0, but the system will have no space for running apps if they are both running at the same time. You need to allocate 1/3 ram to each at best. That will have be a new compile time option in a further patch, that I will release should someone make the argument for wanting both of these. You either want 1 of 3 possible setups. #1 This patch. Best for people who need to opkg -d ram a lot of stuff #2 tmpfs and progs BOTH can use zram-swap which makes it a better overall choice, after thinking about this patch it isn't worth it as much as just using zram-swap. But better have a working feature than a broken one as it exists in buildroot now Or #3 Assuming you have a fast storage device connected to your openwrt (most don't but some NASes do) ZSWAP kernel will dynamically use free memory to offer a pre-swap cache, and allow compressed data (lzo/lz4) to be wrote to swap. It may require an actual initialized swap device to work. I haven't RTFMed yet. 3) Debatable: LZ4 should be on by default. (My two cents) Justification: It's almost compresses well as LZO but it's less CPU intensive for those without Floating point SMID's which is most home routers. So put some help about that in the Makefile. LZ4 is included as a dependency to the zram.ko module and will be loaded at early boot by modprobe / procd. Contrary to rumors circulating on openwrt's trac it actually works :) https://dev.openwrt.org/ticket/22666#comment:3 4) Highly recommended: Makefile should block option if your kernel isn't 4.4. Justification: https://dev.openwrt.org/ticket/21705 & above 5) Stop openwrt folks trying to patch the kernel to make two zram devices by default because you can hot-add them. https://dev.openwrt.org/ticket/19586#comment:10 Here's the patch --------------------------------------------- procd: remove dependency on bloated e2fsprogs/libext2 and use CONFIG_BUSYBOX_CONFIG_MKFS_EXT2=y At a minimum (see accompanying email on mailing list) advising my two cents on the pros and cons of this patch you need to add a configuration option to the Makefile if you want lz4 support config PROCD_ZRAM_TMPFS bool default n prompt "Mount /tmp using zram." endmenu & ifeq ($(CONFIG_PROCD_ZRAM_TMPFS_LZ4),y) CMAKE_OPTIONS += -DZRAM_TMPFS_LZ4=1 endif Signed-off-by: Luke McKee <hojur...@gmail.com> --- a/initd/zram.c 2016-07-03 06:39:51.011999930 +0700 +++ b/initd/zram.c 2016-07-03 07:00:34.143492847 +0700 @@ -82,7 +82,7 @@ int mount_zram_on_tmp(void) { - char *mkfs[] = { "/usr/sbin/mkfs.ext4", "-b", "4096", "-F", "-L", "TEMP", "-m", "0", "/dev/zram0", NULL }; + char *mkfs[] = { "/sbin/mkfs.ext2", "-b", "4096", "-F", "-L", "TEMP", "-m", "0", "/dev/zram0", NULL }; FILE *fp; long zramsize; pid_t pid; @@ -94,6 +95,15 @@ } mkdev("*", 0600); +#ifdef ZRAM_TMPFS_LZ4 + fp = fopen("/sys/block/zram0/comp_algorithm", "r+"); + if (fp == NULL) { + ERROR("Can't open /sys/block/zram0/comp_algorithm: %s\n", strerror(errno)); + return errno; + } + fprintf(fp, "%s", "lz4" ); + fclose(fp); +#endif zramsize = proc_meminfo() / 2; fp = fopen("/sys/block/zram0/disksize", "r+"); @@ -107,10 +116,10 @@ pid = fork(); if (!pid) { execvp(mkfs[0], mkfs); - ERROR("Can't exec /sbin/mkfs.ext4\n"); + ERROR("Can't exec /sbin/mkfs.ext2\n"); exit(-1); } else if (pid <= 0) { - ERROR("Can't exec /sbin/mkfs.ext4\n"); + ERROR("Can't exec /sbin/mkfs.ext2\n"); return -1; } else { waitpid(pid, NULL, 0); _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel