[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5aa1e8c55cf84436b35ee5557a508e8d@AcuMS.aculab.com>
Date: Sat, 8 Jan 2022 22:35:24 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Pintu Agarwal' <pintu.ping@...il.com>
CC: Pintu Kumar <quic_pintu@...cinc.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"ebiederm@...ssion.com" <ebiederm@...ssion.com>,
"christian.brauner@...ntu.com" <christian.brauner@...ntu.com>,
"sfr@...b.auug.org.au" <sfr@...b.auug.org.au>,
"legion@...nel.org" <legion@...nel.org>,
"sashal@...nel.org" <sashal@...nel.org>,
"gorcunov@...il.com" <gorcunov@...il.com>,
"chris.hyser@...cle.com" <chris.hyser@...cle.com>,
"ccross@...gle.com" <ccross@...gle.com>,
"pcc@...gle.com" <pcc@...gle.com>,
"dave@...olabs.net" <dave@...olabs.net>,
"caoxiaofeng@...ong.com" <caoxiaofeng@...ong.com>,
"david@...hat.com" <david@...hat.com>,
"vbabka@...e.cz" <vbabka@...e.cz>,
"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
"dhowells@...hat.com" <dhowells@...hat.com>
Subject: RE: [PATCH v2] sysinfo: include availram field in sysinfo struct
From: Pintu Agarwal
> Sent: 08 January 2022 16:53
>
> On Sat, 8 Jan 2022 at 03:52, David Laight <David.Laight@...lab.com> wrote:
> >
> > From: Pintu Kumar
> > > Sent: 07 January 2022 18:08
> > >
> > > 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.
> > >
> > > Thus, this patch introduce a new field as availram in sysinfo
> > > so that all the info total/free/available can be retrieved from
> > > one place itself.
> > >
> > ...
> > > diff --git a/include/uapi/linux/sysinfo.h b/include/uapi/linux/sysinfo.h
> > > index 435d5c2..fe84c6a 100644
> > > --- a/include/uapi/linux/sysinfo.h
> > > +++ b/include/uapi/linux/sysinfo.h
> > > @@ -19,7 +19,8 @@ struct sysinfo {
> > > __kernel_ulong_t totalhigh; /* Total high memory size */
> > > __kernel_ulong_t freehigh; /* Available high memory size */
> > > __u32 mem_unit; /* Memory unit size in bytes */
> > > - char _f[20-2*sizeof(__kernel_ulong_t)-sizeof(__u32)]; /* Padding: libc5 uses this.. */
> >
> > There are 4 pad bytes here on most 64bit architectures.
> >
> > > + __kernel_ulong_t availram; /* Memory available for allocation */
> > > + char _f[20-3*sizeof(__kernel_ulong_t)-sizeof(__u32)]; /* Padding: libc5 uses this.. */
> > > };
> >
> > You've not compile-time tested the size of the structure.
> >
> With "32" instead of "20" in padding I get these size of sysinfo:
> In x86-64 kernel, with app 64-bit: Size of sysinfo = 128
> In x86-64 kernel, with app 32-bit:: Size of sysinfo = 76
> In arm-64 kernel, with app 32-bit: Size of sysinfo = 76
You need to compare the sizes before and after your patch
to ensure it doesn't change on any architecture.
> Okay the sys robot reported some issue in 64-bit build.
> {{{
> >> include/uapi/linux/sysinfo.h:23:14: error: size of array '_f' is too large
> >> 23 | char _f[20-3*sizeof(__kernel_ulong_t)-sizeof(__u32)]; /* Padding: libc5 uses
> this.. */
> >> | ^~
> }}}
>
> Also, I got the same issue while building for arm64, so I tried to
> adjust like this:
> char _f[32-3*sizeof(__kernel_ulong_t)-sizeof(__u32)];
>
> With this the build works on both 32/64 but output fails when running
> 32-bit program on 64-bit kernel.
> Also, the free command on 64-bit reports "stack smashing error"..
>
> How do we resolve this issue to make it work on both arch ?
> Also, I don't really understand the significance of that number "20"
> in padding ?
My guess is that someone added a char _f[20] pad to allow for expansion.
Then two __kernel_ulong_t and one __u32 field were added, so the
size of the pad was reduced.
When __kernel_ulong_t is 64bit then it seems to be char _f[0]
- which might generate a compile warning since you are supposed
to use char _f[] to indicate an extensible structure.
There is, however, 4 bytes of pad after the _f[] on most 64bit
architectures.
So actually there isn't enough space to anything useful at all.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Powered by blists - more mailing lists