linux-kernel - Re: [PATCH v2] sysinfo: include availram field in sysinfo struct

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOuPNLhcvbk3-rTPqmJj5LBOh4VaZ+Bc=-_j6xKOLM-kH6jkfw@mail.gmail.com>
Date:   Sat, 8 Jan 2022 21:54:37 +0530
From:   Pintu Agarwal <pintu.ping@...il.com>
To:     Cyrill Gorcunov <gorcunov@...il.com>
Cc:     Pintu Kumar <quic_pintu@...cinc.com>,
        open list <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-mm <linux-mm@...ck.org>, ebiederm@...ssion.com,
        Christian Brauner <christian.brauner@...ntu.com>,
        sfr@...b.auug.org.au, legion@...nel.org, sashal@...nel.org,
        chris.hyser@...cle.com, Colin Cross <ccross@...gle.com>,
        Peter Collingbourne <pcc@...gle.com>, dave@...olabs.net,
        caoxiaofeng@...ong.com, david@...hat.com,
        Vlastimil Babka <vbabka@...e.cz>, linux-api@...r.kernel.org
Subject: Re: [PATCH v2] sysinfo: include availram field in sysinfo struct

On Sat, 8 Jan 2022 at 02:31, Cyrill Gorcunov <gorcunov@...il.com> wrote:
>
> On Fri, Jan 07, 2022 at 11:37:34PM +0530, Pintu Kumar wrote:
> > The sysinfo member does not have any "available ram" field and
> > the bufferram field is not much helpful either, to get a rough
> > estimate of available ram needed for allocation.
> >
> > One needs to parse MemAvailable field separately from /proc/meminfo
> > to get this info instead of directly getting if from sysinfo itself.
>
> Who exactly needs this change? Do you have some application for which
> parsing /proc/meminfo is a hot path so it needs this information via
> sysinfo interface?
>
Thank you so much for your feedback...
I had a need to get total/free/available memory in my application (on
a memory constraint system).
First I tried to parse these from /proc/meminfo but then I saw sysinfo
already provides some information,
however available field was missing. Just to get available field I
need to again do all the file operations.

Then I saw, even the "free" command doing this redundant work.
Use sysinfo system call to get "total" and "free" memory then again
get "available" memory from /proc/meminfo.
Yet again, I see, even in kernel its reading from two different places
while populating the /proc/meminfo.
Also, I am sure there are plenty of other applications where this can
be improved with this.
Moreover, I think with this field there is not much use of other ram
fields in sysinfo.
Thus I felt a need to introduce this field to ease some operations.

> Don't get me wrong please but such extension really need a strong
> justification because they are part of UAPI and there is not that much
> space left in sysinfo structure. We will _have_ to live with this new
> field forever so I propose to not introduce anything new here until
> we have no other choise or parsing meminfo become a really bottleneck.
>
My guess is that this situation might exist in other places as well ?
How do we handle new field addition to existing system calls ?

> > diff --git a/kernel/sys.c b/kernel/sys.c
> > index ecc4cf0..7059515 100644
> > --- a/kernel/sys.c
> > +++ b/kernel/sys.c
> > @@ -2671,6 +2671,7 @@ static int do_sysinfo(struct sysinfo *info)
> >       info->freeram <<= bitcount;
> >       info->sharedram <<= bitcount;
> >       info->bufferram <<= bitcount;
> > +     info->availram <<= bitcount;
> >       info->totalswap <<= bitcount;
> >       info->freeswap <<= bitcount;
> >       info->totalhigh <<= bitcount;
> > @@ -2700,6 +2701,7 @@ struct compat_sysinfo {
> >       u32 freeram;
> >       u32 sharedram;
> >       u32 bufferram;
> > +     u32 availram;
>
> If only I'm not missing something ovious, this is part of UAPI as well.
Yes, this structure is part of the common UAPI header.