From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 918B5C433EF for ; Fri, 7 Jan 2022 13:44:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E7DA36B0071; Fri, 7 Jan 2022 08:44:22 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E38D66B0073; Fri, 7 Jan 2022 08:44:22 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CA6C06B0074; Fri, 7 Jan 2022 08:44:22 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0207.hostedemail.com [216.40.44.207]) by kanga.kvack.org (Postfix) with ESMTP id B5A436B0071 for ; Fri, 7 Jan 2022 08:44:22 -0500 (EST) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 716E0181951DD for ; Fri, 7 Jan 2022 13:44:22 +0000 (UTC) X-FDA: 79003610364.01.CD68DC0 Received: from mail-yb1-f176.google.com (mail-yb1-f176.google.com [209.85.219.176]) by imf09.hostedemail.com (Postfix) with ESMTP id 26C66140007 for ; Fri, 7 Jan 2022 13:44:22 +0000 (UTC) Received: by mail-yb1-f176.google.com with SMTP id p15so16791971ybk.10 for ; Fri, 07 Jan 2022 05:44:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=cShoU76vrbE0mQ8bMH8Dhj79NbR2cnrdoCS7NmG1tqk=; b=dm3TPOKjEpcXX2HP4tlgHyCVDjM7Q4pKjcoe4YloY8UhVRkPfB8PgNfZg/OWREQmqd LEzLYkLvLtoPU5HZlGKkOhUtpCzLcNdEQDUilT7iMuWnjLToxQxqvH4EJCF62umcVqiG w2j1vG81y5hKKJBVaooqQsU01jY6hanXeK72PnIIqmxgk06Wwd1oZmd1CWfq3pl00cht 6LQ6xMK3jIGNrVzaargYENIfHfEWEoSVZkVFI/v5m1ksOWmWG13ZD2Yck3Zi3YIWYE/c B+rLblSImExoo1VLH/u0F5gwN28/V9qgJFH05sPEjLCLzJOGQVfN5LEWsiiRL/JojDv6 Nt+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=cShoU76vrbE0mQ8bMH8Dhj79NbR2cnrdoCS7NmG1tqk=; b=beEk7CZtDTELUGENbZ6JSBkMifdni8sMOTAHDc0Gcy7BSUZq2bhRvnJYFH1plz27zv Isu6ZGXvWQINdi2PAC8TWATmqE+Ptqh+AUND/8GcSIz5mvgR6O/mEF1zf5iJ6XtPamXU zQdyk6DIC3XLX2xsXzAMwvQPhDcaLDUF0M35ihe1OZ0fFNdMJUsssg1YVgiILyGJvSoA T/mycwqYvK7U1GvIjodUPHwF7+7Yr8q7yIBW6LwkErWW1vV0N3ktwZyzWJ3Z83b6sUZf lY9M50tvU0UeLluoTDLAXZv2hucRYiLSU+af3SvnmR+mUwTjWmyVpKw8dpqZXl1uZmAx 1MtA== X-Gm-Message-State: AOAM532fVWYcaUTcmEpbax8/YgkJE5Y1EwS8Q5sQEXMF9u90vKVHmagQ Tad7IHwzJJFIV0lK/mGXyc+UA4K2DtQO2aMRM2k= X-Google-Smtp-Source: ABdhPJyfEqvU6xnJsvTMPjXvjABY9UNAgoQdbxjwoYCNPCdd/98CB6eh73UsYhZ9Z2uSQFrYJxqKkDmq1CR0ucerCdE= X-Received: by 2002:a25:8b8f:: with SMTP id j15mr79841190ybl.585.1641563061275; Fri, 07 Jan 2022 05:44:21 -0800 (PST) MIME-Version: 1.0 References: <1641483250-18839-1-git-send-email-quic_pintu@quicinc.com> <20220107120451.z2eqru2tm5mlhla3@wittgenstein> In-Reply-To: <20220107120451.z2eqru2tm5mlhla3@wittgenstein> From: Pintu Agarwal Date: Fri, 7 Jan 2022 19:14:10 +0530 Message-ID: Subject: Re: [PATCH] sysinfo: include availram field in sysinfo struct To: Christian Brauner Cc: Cyrill Gorcunov , Pintu Kumar , open list , Andrew Morton , linux-mm , ebiederm@xmission.com, sfr@canb.auug.org.au, legion@kernel.org, sashal@kernel.org, chris.hyser@oracle.com, ccross@google.com, pcc@google.com, dave@stgolabs.net, caoxiaofeng@yulong.com, david@redhat.com Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 26C66140007 X-Stat-Signature: weqjgbssggp6bks9nmtrsfnhsqa4r48u Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=dm3TPOKj; spf=pass (imf09.hostedemail.com: domain of pintu.ping@gmail.com designates 209.85.219.176 as permitted sender) smtp.mailfrom=pintu.ping@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-HE-Tag: 1641563062-961473 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, 7 Jan 2022 at 17:35, Christian Brauner wrote: > > On Thu, Jan 06, 2022 at 08:27:47PM +0300, Cyrill Gorcunov wrote: > > On Thu, Jan 06, 2022 at 10:19:55PM +0530, Pintu Agarwal wrote: > > > > > diff --git a/include/uapi/linux/sysinfo.h b/include/uapi/linux/sysinfo.h > > > > > index 435d5c2..6e77e90 100644 > > > > > --- a/include/uapi/linux/sysinfo.h > > > > > +++ b/include/uapi/linux/sysinfo.h > > > > > @@ -12,6 +12,7 @@ struct sysinfo { > > > > > __kernel_ulong_t freeram; /* Available memory size */ > > > > > __kernel_ulong_t sharedram; /* Amount of shared memory */ > > > > > __kernel_ulong_t bufferram; /* Memory used by buffers */ > > > > > + __kernel_ulong_t availram; /* Memory available for allocation */ > > > > > __kernel_ulong_t totalswap; /* Total swap space size */ > > > > > __kernel_ulong_t freeswap; /* swap space still available */ > > > > > __u16 procs; /* Number of current processes */ > > > > > > > > Hi! Sorry, but I don't understand -- the sysinfo structure seems to > > > > be part of user API, no? Don't we break it up here? > > > > > > Yes, the corresponding user space header /usr/include/linux/sysinfo.h > > > also needs to be updated. > > > When we generate the kernel header it will be updated automatically. > > > > Wait. The userspace may pass old structure here, and in result we > > return incorrect layout which won't match old one, no? Old binary > > code has no clue about this header update. > > Yes, that won't work as done. > > If we want to do this it needs to be done at the end of the struct right > before the padding field and the newly added field substracted from the > padding. (Not the preferred way to do it these days for new structs.) > > A new kernel can then pass in the struct with the newly added field and > an old kernel can just fill the struct in as usual. New kernel will > update the field with the correct value. > > But there's a catch depending on the type of value. > The problem with these types of extensions is that you'll often need > indicators to and from the kernel whether the extension is supported. > > Consider an extension where 0 is a valid value meaning "this resource is > completely used". Since the kernel and userspace always agree on the > size of the struct the kernel will zero the whole struct. So if in your > newly added field 0 is a valid value you can't differentiate between 0 > as a valid value indicating that your resource isn't available and 0 as > the kernel not supporting your extension. > > Other APIs solve this and similar problems by having a request mask and > a return mask. Userspace fills in what values it wants reported in the > request mask and kernel sets the supported flags in the return mask. > This way you can differentiate between the two (see statx). > > If the 0 example is not a concern or acceptable for userspace it's > probably fine. But you need to document that having 0 returned can mean > both things. > > Or, you select a value different from 0 (-1?) that you can use to > indicate to userspace that the resource is used up so 0 can just mean > "kernel doesn't support this extension". Thanks all for your inputs. As Eric suggested in other thread (pasting here for reference): { > Before the padding and you should reduce the size of the padding by the > size of your new field. >> Also, I could not understand what this is for ? >> Do we need to update this since sture is changed ? > In general padding like that is so new fields can be added. The > comment about libc5 makes me a wonder a bit, but I expect libc5 just > added the padding in it's copy of the structure that it exported to > userspace many many years ago so that new fields could be added. > Eric } I made the changes like below and this seems to work even with older user space. I mean earlier, when I ran "free" command it was giving "stack smashing..." error, but now the "free" command (which comes as part of busybox) works fine even without recompiling with the updated header. These are the header changes for quick look: {{{ diff --git a/include/uapi/linux/sysinfo.h b/include/uapi/linux/sysinfo.h index 6e77e90..fe84c6a 100644 --- a/include/uapi/linux/sysinfo.h +++ b/include/uapi/linux/sysinfo.h @@ -12,7 +12,6 @@ struct sysinfo { __kernel_ulong_t freeram; /* Available memory size */ __kernel_ulong_t sharedram; /* Amount of shared memory */ __kernel_ulong_t bufferram; /* Memory used by buffers */ - __kernel_ulong_t availram; /* Memory available for allocation */ __kernel_ulong_t totalswap; /* Total swap space size */ __kernel_ulong_t freeswap; /* swap space still available */ __u16 procs; /* Number of current processes */ @@ -20,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.. */ + __kernel_ulong_t availram; /* Memory available for allocation */ + char _f[20-3*sizeof(__kernel_ulong_t)-sizeof(__u32)]; /* Padding: libc5 uses this.. */ }; }}} If this is fine, I will push the new patch set. Thanks, Pintu