The code that wmmon uses to read /proc/meminfo assumes an incorrect format. This results in bad information being loaded into various data structures. Additionally, there apears to be an issue with calculating percent used where the calculation is ( total * 100 ) / free. Reproducible: Always Steps to Reproduce: 1. emerge wmmon 2. 3.
Created attachment 56297 [details, diff] Patch to fix bug I have corrected bug with stat gathering. I have also corrected the bug in % usage calculations. Other changes are removing a deprecated function; properly naming stats_device[]s according to their usage; streamlining code around reading stats for memory and swap usage and calculating / drawing usage bars.
Just a quick question: your patch adds support for 2.6 kernels, but is it backwards compatible with 2.4 ones?
I have not tested this, but runing `cat /proc/meminfo` on a machine running a 2.4 kernel leads me to believ that yes, it is backwards compatible. It appears that 2.4 kernels had a couple of lines that had all pertinent Memory and Swap information on them plus individual lines with Total and Free stats. 2.6 kernels only contain the individual stat lines. My patch will just read those lines. There is one question that I have. The previous version of wmmon subtracts Buffers and Cache from the used memory. My patch does not. I am of the opinion that Buffers and Cache are indeed allocated / used memory and therefore should be included in the accounting. One of the original developers, evidently, thought that Buffers and Cache should not be included. What are the opinions of some of the others who may use this app? Perhaps I should put a switch on the command to turn this feature off/on? Suggestions?
Comment on attachment 56297 [details, diff] Patch to fix bug >*** wmmon.c Thu Apr 14 13:31:14 2005 >--- wmmon.fixed Thu Apr 14 13:31:20 2005 >*************** void DrawActive(char *); >*** 188,194 **** > void update_stat_cpu(stat_dev *); > void update_stat_io(stat_dev *); > void update_stat_mem(stat_dev *st, stat_dev *st2); >- void update_stat_swp(stat_dev *); > > void wmmon_routine(int argc, char **argv) { > >--- 188,193 ---- >*************** void wmmon_routine(int argc, char **argv >*** 298,304 **** > > if(stat_current == 2) { > update_stat_mem(&stat_device[2], &stat_device[3]); >- // update_stat_swp(&stat_device[3]); > } > > if (stat_current < 2) { >--- 297,302 ---- >*************** void wmmon_routine(int argc, char **argv >*** 320,341 **** > copyXPMArea(0, 64, 32, 12, 28+64, 4); > copyXPMArea(0, 64, 32, 12, 28+64, 18); > >! j = stat_device[2].rt_idle; >! if (j != 0) { >! j = (stat_device[2].rt_stat * 100) / j; >! } >! j = j * 0.32; >! if (j > 32) j = 32; >! copyXPMArea(32, 64, j, 12, 28+64, 4); >! /*--------------------- ------------------*/ >! j = stat_device[3].rt_idle; >! if (j != 0) { >! j = (stat_device[3].rt_stat * 100) / j; > } >- j = j * 0.32; >- if (j > 32) j = 32; >- copyXPMArea(32, 64, j, 12, 28+64, 18); >- > /*----------- online tijd neerzetten! ----------*/ > > cnt_time = time(0) - ref_time + online_time; >--- 318,331 ---- > copyXPMArea(0, 64, 32, 12, 28+64, 4); > copyXPMArea(0, 64, 32, 12, 28+64, 18); > >! for (i = 2; i <= 3; i++) { >! j = stat_device[i].rt_stat != 0 >! ? 100 * ( stat_device[i].rt_stat - stat_device[i].rt_idle ) / stat_device[i].rt_stat >! : 0 ; >! j = j * 0.32; >! if (j > 32) j = 32; >! copyXPMArea(32, 64, j, 12, 28+64, 4+(14*(i-2)) ); > } > /*----------- online tijd neerzetten! ----------*/ > > cnt_time = time(0) - ref_time + online_time; >*************** void wmmon_routine(int argc, char **argv >*** 430,436 **** > } > case 1: > stat_current++; >- printf("current stat is :%d\n", stat_current); > if (stat_current == stat_online) > stat_current = 0; > >--- 420,425 ---- *************** void update_stat_io(stat_dev *st) { *** 499,541 **** void update_stat_mem(stat_dev *st, stat_dev *st2) { ! char temp[128]; ! unsigned long free, shared, buffers, cached; freopen("/proc/meminfo", "r", fp_meminfo); while (fgets(temp, 128, fp_meminfo)) { ! if (strstr(temp, "Mem:")) { ! sscanf(temp, "Mem: %ld %ld %ld %ld %ld %ld", ! &st->rt_idle, &st->rt_stat, ! &free, &shared, &buffers, &cached); ! st->rt_idle >>= 10; ! st->rt_stat -= buffers+cached; ! st->rt_stat >>= 10; ! // break; } ! if (strstr(temp, "Swap:")) { ! sscanf(temp, "Swap: %ld %ld", &st2->rt_idle, &st2->rt_stat); ! st2->rt_idle >>= 10; ! st2->rt_stat >>= 10; ! break; } ! } ! } ! ! void update_stat_swp(stat_dev *st) { ! ! char temp[128]; ! ! fseek(fp_meminfo, 0, SEEK_SET); ! while (fgets(temp, 128, fp_meminfo)) { ! if (strstr(temp, "Swap:")) { ! sscanf(temp, "Swap: %ld %ld", &st->rt_idle, &st->rt_stat); ! st->rt_idle >>= 10; ! st->rt_stat >>= 10; ! break; } } - } /****************************************************************************** *\ --- 488,512 ---- void update_stat_mem(stat_dev *st, stat_dev *st2) { ! char temp[128], stat[20]; ! unsigned long amount; freopen("/proc/meminfo", "r", fp_meminfo); while (fgets(temp, 128, fp_meminfo)) { ! sscanf(temp, "%s %ld", &stat, &amount); ! if (! strcmp(stat, "MemFree:") ) { ! st->rt_idle = (amount >> 10); } ! else if (! strcmp(stat, "MemTotal:") ) { ! st->rt_stat = (amount >> 10); } ! else if (! strcmp(stat, "SwapFree:") ) { ! st2->rt_idle = (amount >> 10); ! } ! else if (! strcmp(stat, "SwapTotal:") ) { ! st2->rt_stat = (amount >> 10); } } } /****************************************************************************** *\ *************** int checksysdevs(void) { *** 599,605 **** strcpy(stat_device[0].name, "cpu0"); strcpy(stat_device[1].name, "i/o"); ! strcpy(stat_device[2].name, "sys"); return 3; } --- 570,577 ---- strcpy(stat_device[0].name, "cpu0"); strcpy(stat_device[1].name, "i/o"); ! strcpy(stat_device[2].name, "mem"); ! strcpy(stat_device[3].name, "swap"); return 3; }
I found a mistake in the patch I submitted that resulted in Swap info being drawn 4 pixels to low. I tried updating the patch, which resulted in the previous comment. I have another patch that fixes this mistake, but also adds a '-b' switch to subtract Buffers and Cache from the used memory.
Created attachment 56594 [details, diff] Fix bug; add '-b' option
Uh, sorry for the late reply... Buffer and cache should not be considered when counting used memory (or maybe counted using different colours), because when extra RAM is needed for apps, the kernel shrinks them as needed (to the point of making them 0 in size if necessary)... Ah, you forgot to add info of the existence of '-b' cmd line arg in the output of 'wmmon -h'...
Created attachment 58618 [details, diff] swap default behavior of '-b'; add '-b' to usage I've added a description of the '-b' option to the usage() function. I also swapped the default behavior to subtract buffers and cache by default; -b turns this off.
You did a nice work here, Ivan :-) Just committed a new revision with your patch to Portage...
