Re: [PATCH] free: Add 'available' memory if provided by the kernel
On Tue, Oct 30, 2018 at 2:46 PM Guillermo Rodriguez Garcia
wrote:
>
> 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.
That's larger code. I don't think users of "free" are that
performance-sensitive.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] free: Add 'available' memory if provided by the kernel
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
() escribió:
>
> Applied, thanks
> On Thu, Oct 4, 2018 at 4:27 PM Guillermo Rodríguez
> 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
> > ---
> > 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", ) == 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(_kb, _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
Re: [PATCH] free: Add 'available' memory if provided by the kernel
Applied, thanks
On Thu, Oct 4, 2018 at 4:27 PM Guillermo Rodríguez
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
> ---
> 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", ) == 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(_kb, _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
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
