2022-04-14 18:51:27

by Alex Xu (Hello71)

[permalink] [raw]
Subject: [PATCH] mm/smaps_rollup: return empty file for kthreads instead of ESRCH

This restores the behavior prior to 258f669e7e88 ("mm:
/proc/pid/smaps_rollup: convert to single value seq_file"), making it
once again consistent with maps and smaps, and allowing patterns like
awk '$1=="Anonymous:"{x+=$2}END{print x}' /proc/*/smaps_rollup to work.
Searching all Debian packages for "smaps_rollup" did not find any
programs which would be affected by this change.

Fixes: 258f669e7e88 ("mm: /proc/pid/smaps_rollup: convert to single value seq_file")

Signed-off-by: Alex Xu (Hello71) <[email protected]>
---
fs/proc/task_mmu.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index f46060eb91b5..d7de4584a271 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -883,10 +883,8 @@ static int show_smaps_rollup(struct seq_file *m, void *v)
return -ESRCH;

mm = priv->mm;
- if (!mm || !mmget_not_zero(mm)) {
- ret = -ESRCH;
+ if (!mm || !mmget_not_zero(mm))
goto out_put_task;
- }

memset(&mss, 0, sizeof(mss));

--
2.35.2


2022-04-15 07:41:35

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH] mm/smaps_rollup: return empty file for kthreads instead of ESRCH

On Thu, Apr 14, 2022 at 04:23:13AM +0100, Matthew Wilcox wrote:
> On Wed, Apr 13, 2022 at 04:06:13PM -0700, Andrew Morton wrote:
> > On Wed, 13 Apr 2022 18:25:53 -0400 "Alex Xu (Hello71)" <[email protected]> wrote:
> > > > 258f669e7e88 was 4 years ago, so I guess a -stable backport isn't
> > > > really needed.
> > >
> > > Current behavior (4.19+):
> [...]
> > > Pre-4.19 and post-patch behavior:
> >
> > I don't think this will work very well. smaps_rollup is the sort of
> > system tuning thing for which organizations will develop in-house
> > tooling which never get relesaed externally.
> >
> > > 3. As mentioned previously, this was already the behavior between 4.14
> > > and 4.18 (inclusive).
> > >
> >
> > Yup. Hm, tricky. I'd prefer to leave it alone if possible. How
> > serious a problem is this, really?
>
> I don't think "It's been like this for four years" is as solid an argument
> as you might like. Certain distributions (of the coloured millinery
> variety, for example) haven't updated their kernel since then and so
> there may well be many organisations who have not been exposed to the
> current behaviour. Even my employers distribution, while it offers a
> 5.4 based kernel, still has many customers who have not moved from the
> 4.14 kernel. Inertia is a real thing, and restoring this older behaviour
> might well be an improvement.

Returning ESRCH is better so that programs don't waste time reading and
closing empty files and instantiating useless inodes.

Of course it is different if this patch was sent as response to a regression.

2022-04-16 02:16:47

by Alex Xu (Hello71)

[permalink] [raw]
Subject: Re: [PATCH] mm/smaps_rollup: return empty file for kthreads instead of ESRCH

Excerpts from Alexey Dobriyan's message of April 14, 2022 2:55 am:
> Returning ESRCH is better so that programs don't waste time reading and
> closing empty files and instantiating useless inodes.

Yes, except ESRCH is not returned for open, it is returned for read.

> Of course it is different if this patch was sent as response to a regression.

I'm not sure I would classify it as a regression; I don't have an
existing program which broke, it is a new program which happens to use
some functionality which worked with a previous kernel. It is
theoretically possible that some program exists that currently uses
4.14, and will break if upgraded to 4.19+, but it is also possible that
some program exists that currently uses 4.19+ and will break if this
patch is applied.

Cheers,
Alex.