Thank you.
One minor thing; in the code that has been commited, the seen_cached
and seen_available vars have been combined into one
(seen_cached_and_available). However the code now parses all lines in
/proc/meminfo twice until both Cached and MemAvailable have been
found:
seen_cached_and_available = 2;
while (fgets(buf, sizeof(buf), fp)) {
if (sscanf(buf, "Cached: %lu %*s\n", cached_kb) == 1)
if (--seen_cached_and_available == 0)
break;
if (sscanf(buf, "MemAvailable: %lu %*s\n", available_kb) == 1)
if (--seen_cached_and_available == 0)
break;
}
However the original code only checks for things it has not seen yet;
for example if it finds "Cached" then it will only check for
MemAvailable in the remaining lines:
while (fgets(buf, sizeof(buf), f) != NULL && !(seen_cached &&
seen_available)) {
if (!seen_cached && sscanf(buf, "Cached: %lu %*s\n",
cached_kb) == 1) {
seen_cached = 1;
}
else if (!seen_available && sscanf(buf, "MemAvailable:
%lu %*s\n", available_kb) == 1) {
seen_available = 1;
}
}
I think it would be convenient to revert this change and keep the two
separate variables in order to avoid doing unnecessary work.
Best,
Guillermo
El mar., 30 oct. 2018 a las 14:15, Denys Vlasenko
(<[email protected]>) escribió:
>
> Applied, thanks
> On Thu, Oct 4, 2018 at 4:27 PM Guillermo Rodríguez
> <[email protected]> wrote:
> >
> > Show estimated available memory if this is provided by the
> > kernel. See [1] for the full story.
> >
> > [1]:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=34e431b0ae398fc54ea69ff85ec700722c9da773
> >
> > Signed-off-by: Guillermo Rodriguez <[email protected]>
> > ---
> > procps/free.c | 54 ++++++++++++++++++++++++++++++++++--------------------
> > 1 file changed, 34 insertions(+), 20 deletions(-)
> >
> > diff --git a/procps/free.c b/procps/free.c
> > index 0991713..e7ba974 100644
> > --- a/procps/free.c
> > +++ b/procps/free.c
> > @@ -44,27 +44,36 @@ static unsigned long long scale(unsigned long d)
> > return ((unsigned long long)d * G.mem_unit) >> G_unit_steps;
> > }
> >
> > -static unsigned long parse_cached_kb(void)
> > +static unsigned int parse_meminfo(unsigned long *cached_kb, unsigned long
> > *available_kb)
> > {
> > char buf[60]; /* actual lines we expect are ~30 chars or less */
> > FILE *f;
> > - unsigned long cached = 0;
> > + int seen_cached = 0;
> > + int seen_available = 0;
> > +
> > + *cached_kb = *available_kb = 0;
> >
> > f = xfopen_for_read("/proc/meminfo");
> > - while (fgets(buf, sizeof(buf), f) != NULL) {
> > - if (sscanf(buf, "Cached: %lu %*s\n", &cached) == 1)
> > - break;
> > + while (fgets(buf, sizeof(buf), f) != NULL && !(seen_cached &&
> > seen_available)) {
> > + if (!seen_cached && sscanf(buf, "Cached: %lu %*s\n",
> > cached_kb) == 1) {
> > + seen_cached = 1;
> > + }
> > + else if (!seen_available && sscanf(buf, "MemAvailable: %lu
> > %*s\n", available_kb) == 1) {
> > + seen_available = 1;
> > + }
> > }
> > fclose(f);
> >
> > - return cached;
> > + return seen_available;
> > }
> >
> > int free_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
> > int free_main(int argc UNUSED_PARAM, char **argv
> > IF_NOT_DESKTOP(UNUSED_PARAM))
> > {
> > struct sysinfo info;
> > - unsigned long long cached;
> > + unsigned long long cached, available;
> > + unsigned long cached_kb, available_kb;
> > + int seen_available;
> >
> > INIT_G();
> >
> > @@ -95,17 +104,19 @@ int free_main(int argc UNUSED_PARAM, char **argv
> > IF_NOT_DESKTOP(UNUSED_PARAM))
> > /* Kernels prior to 2.4.x will return info.mem_unit==0, so cope...
> > */
> > G.mem_unit = (info.mem_unit ? info.mem_unit : 1);
> >
> > - /* Extract cached from /proc/meminfo and convert to mem_units */
> > - cached = ((unsigned long long) parse_cached_kb() * 1024) /
> > G.mem_unit;
> > + /* Extract cached and memavailable from /proc/meminfo and convert
> > to mem_units */
> > + seen_available = parse_meminfo(&cached_kb, &available_kb);
> > + cached = ((unsigned long long) cached_kb * 1024) / G.mem_unit;
> > + available = ((unsigned long long) available_kb * 1024) / G.mem_unit;
> >
> > - printf(" %11s%11s%11s%11s%11s%11s\n",
> > + printf(" %12s%12s%12s%12s%12s%12s\n",
> > "total",
> > "used",
> > "free",
> > - "shared", "buffers", "cached" /* swap and total don't have
> > these columns */
> > + "shared", "buff/cache", "available" /* swap and total don't
> > have these columns */
> > );
> >
> > -#define FIELDS_6 "%11llu%11llu%11llu%11llu%11llu%11llu\n"
> > +#define FIELDS_6 "%12llu%12llu%12llu%12llu%12llu%12llu\n"
> > #define FIELDS_3 (FIELDS_6 + 3*6)
> > #define FIELDS_2 (FIELDS_6 + 4*6)
> >
> > @@ -115,16 +126,19 @@ int free_main(int argc UNUSED_PARAM, char **argv
> > IF_NOT_DESKTOP(UNUSED_PARAM))
> > scale(info.totalram - info.freeram),
> > scale(info.freeram),
> > scale(info.sharedram),
> > - scale(info.bufferram),
> > - scale(cached)
> > + scale(info.bufferram) + scale(cached),
> > + scale(available)
> > );
> > - /* Show alternate, more meaningful busy/free numbers by counting
> > + /* On kernels < 3.14, MemAvailable is not provided.
> > + * Show alternate, more meaningful busy/free numbers by counting
> > * buffer cache as free memory. */
> > - printf("-/+ buffers/cache:");
> > - printf(FIELDS_2,
> > - scale(info.totalram - info.freeram - info.bufferram -
> > cached),
> > - scale(info.freeram + info.bufferram + cached)
> > - );
> > + if (!seen_available) {
> > + printf("-/+ buffers/cache:");
> > + printf(FIELDS_2,
> > + scale(info.totalram - info.freeram - info.bufferram
> > - cached),
> > + scale(info.freeram + info.bufferram + cached)
> > + );
> > + }
> > #if BB_MMU
> > printf("Swap: ");
> > printf(FIELDS_3,
> > --
> > 1.9.1
> >
> > _______________________________________________
> > busybox mailing list
> > [email protected]
> > http://lists.busybox.net/mailman/listinfo/busybox
--
Guillermo Rodriguez Garcia
[email protected]
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
