Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 13 Apr 2022 18:25:53 -0400
From:   "Alex Xu (Hello71)" <alex_y_xu@yahoo.ca>
Subject: Re: [PATCH] mm/smaps_rollup: return empty file for kthreads instead
 of ESRCH
To:     Andrew Morton <akpm@linux-foundation.org>
Cc:     Alexey Dobriyan <adobriyan@gmail.com>,
        Daniel Colascione <dancol@google.com>,
        linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        Vlastimil Babka <vbabka@suse.cz>
References: <20220413211357.26938-1-alex_y_xu.ref@yahoo.ca>
        <20220413211357.26938-1-alex_y_xu@yahoo.ca>
        <20220413142748.a5796e31e567a6205c850ae7@linux-foundation.org>
In-Reply-To: <20220413142748.a5796e31e567a6205c850ae7@linux-foundation.org>
MIME-Version: 1.0
Message-Id: <1649886492.rqei1nn3vm.none@localhost>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Excerpts from Andrew Morton's message of April 13, 2022 5:27 pm:
> On Wed, 13 Apr 2022 17:13:57 -0400 "Alex Xu (Hello71)" <alex_y_xu@yahoo.c=
a> wrote:
>=20
>> 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=3D=3D"Anonymous:"{x+=3D$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.
>=20
> Thanks.
>=20
> 258f669e7e88 was 4 years ago, so I guess a -stable backport isn't
> really needed.
>=20
> However, we need to be concerned about causing new regressions, and I
> don't think you've presented enough information for this to be determined=
.
>=20
> So please provide us with a full description of how the smaps_rollup
> output will be altered by this patch.  Quoting example output would be
> helpful.
>=20
>=20

Current behavior (4.19+):

$ cat /proc/2/smaps; echo $?
0
$ cat /proc/2/smaps_rollup; echo $?
cat: /proc/2/smaps_rollup: No such process
1
$ strace -yP /proc/2/smaps_rollup cat /proc/2/smaps_rollup
openat(AT_FDCWD</>, "/proc/2/smaps_rollup", O_RDONLY) =3D 3</proc/2/smaps_r=
ollup>
newfstatat(3</proc/2/smaps_rollup>, "", {st_mode=3DS_IFREG|0444, st_size=3D=
0, ...}, AT_EMPTY_PATH) =3D 0
fadvise64(3</proc/2/smaps_rollup>, 0, 0, POSIX_FADV_SEQUENTIAL) =3D 0
read(3</proc/2/smaps_rollup>, 0x7fa475f5d000, 131072) =3D -1 ESRCH (No such=
 process)
cat: /proc/2/smaps_rollup: No such process
close(3</proc/2/smaps_rollup>)          =3D 0
+++ exited with 1 +++

Pre-4.19 and post-patch behavior:

$ cat /proc/2/smaps; echo $?
0
$ cat /proc/2/smaps_rollup; echo $?
0
$ strace -yP /proc/2/smaps_rollup cat /proc/2/smaps_rollup
openat(AT_FDCWD</>, "/proc/2/smaps_rollup", O_RDONLY) =3D 3</proc/2/smaps_r=
ollup>
newfstatat(3</proc/2/smaps_rollup>, "", {st_mode=3DS_IFREG|0444, st_size=3D=
0, ...}, AT_EMPTY_PATH) =3D 0
fadvise64(3</proc/2/smaps_rollup>, 0, 0, POSIX_FADV_SEQUENTIAL) =3D 0
read(3</proc/2/smaps_rollup>, "", 131072) =3D 0
close(3</proc/2/smaps_rollup>)          =3D 0
+++ exited with 0 +++

I agree that this type of change must be done carefully to avoid=20
introducing inadvertent regressions. However, I think this particular=20
change is highly unlikely to introduce regressions for the following=20
reasons:

1. I cannot think of a plausible case which would be affected. The only=20
   case I can possibly imagine is a program checking whether a process=20
   is a kernel thread, but this seems like a particularly silly method.=20
   Moreover, the method is already broken on kernels before 4.14=20
   (because smaps_rollup does not exist) and before 4.19 (because=20
   smaps_rollup worked like smaps). A plausible method would be opening=20
   /proc/x/(s)maps and checking that it is empty, which some programs=20
   actually do.

2. Research on Debian Code Search did not find any apparent cases. I also=20
   searched GitHub Code Search but found too many irrelevant results with=20
   no useful way to filter them out.

3. As mentioned previously, this was already the behavior between 4.14=20
   and 4.18 (inclusive).

Cheers,
Alex.