From: Josh Triplett <josh@joshtriplett.org>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Joey Gouly <joey.gouly@arm.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Alexey Gladkov <legion@kernel.org>,
"Jason A. Donenfeld" <Jason@zx2c4.com>,
Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH] sysinfo: Saturate 16-bit procs rather than wrapping
Date: Thu, 6 Apr 2023 10:03:37 +0900 [thread overview]
Message-ID: <ZC4aac9cO3PrvNrL@localhost> (raw)
In-Reply-To: <87mt3m7ynz.fsf@email.froward.int.ebiederm.org>
On Wed, Apr 05, 2023 at 05:27:12PM -0500, Eric W. Biederman wrote:
> Josh Triplett <josh@joshtriplett.org> writes:
>
> > struct sysinfo has a 16-bit field for the number of processes. Current
> > systems can easily exceed this. Rather than wrapping around, saturate
> > the value at U16_MAX. This is still incorrect, but more likely to
> > help the user know what's going on; a caller can then (for instance)
> > parse the full value out of /proc/loadavg.
> >
> > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> > ---
> >
> > Not sure what tree changes to kernel/sys.c should flow through. Andrew,
> > could you take this through your tree (assuming you agree with it), or
> > suggest what tree it should go through instead?
>
>
> Mind if I ask what the motivation for this is?
[...]
> I looked because just saturating the 16bit field feels like a hack
> that will continue to encourage buggy programs to stay buggy.
On the contrary, it seemed to me like the best way to make issues more
obvious. Wrapping around and showing (say) 12 processes because the
results are mod 65536 won't necessarily immediately stand out in stats,
the way that saturating at 65535 does.
That said, I like the idea of doing 0 even more:
> If there is real value in sysinfo returning a this information someone
> could go through the work and update the kernel to return the high
> bits of the process count in info->pad that is immediately after
> info->procs, and then update the apps or libc to find those high bits.
I'd love to do that; I just thought that'd be less likely to be
accepted. But yes, I think that'd be even better.
That said, I think we need to return *all* the bits in the later
padding, rather than just the high bits, so that we can reliably detect
if the bits were provided. We can return 0 in the existing field, and
then return the process count in the padding, and if the padding is 0
then it wasn't provided.
(If we returned the high bits in the later padding, it'd be hard to tell
if low=42 and high=0 was 42 processes or 42-mod-65536 processes on an
old kernel. If we return the full bits in the later padding, we can
reliably tell the difference between those.)
- Josh Triplett
prev parent reply other threads:[~2023-04-06 1:04 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-02 3:57 [PATCH] sysinfo: Saturate 16-bit procs rather than wrapping Josh Triplett
2023-04-05 22:27 ` Eric W. Biederman
2023-04-06 1:03 ` Josh Triplett [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZC4aac9cO3PrvNrL@localhost \
--to=josh@joshtriplett.org \
--cc=Jason@zx2c4.com \
--cc=akpm@linux-foundation.org \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=ebiederm@xmission.com \
--cc=gregkh@linuxfoundation.org \
--cc=joey.gouly@arm.com \
--cc=legion@kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox