* Re: + selftests-proc-add-proc-pid-statm-output-validation.patch added to mm-nonmm-unstable branch
[not found] <20231004201701.87CB5C433C7@smtp.kernel.org>
@ 2023-10-09 6:14 ` Alexey Dobriyan
2023-10-09 9:00 ` Alexey Dobriyan
2023-10-09 18:16 ` swarup
2023-10-09 19:20 ` [PATCH v3] selftests:proc Add /proc/$(pid)/statm output validation Swarup Laxman Kotiaklapudi
1 sibling, 2 replies; 7+ messages in thread
From: Alexey Dobriyan @ 2023-10-09 6:14 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, shuah, hughd, swarupkotikalapudi
On Wed, Oct 04, 2023 at 01:17:00PM -0700, Andrew Morton wrote:
>
> The patch titled
> Subject: selftests: proc: add /proc/$(pid)/statm output validation
> has been added to the -mm mm-nonmm-unstable branch. Its filename is
> selftests-proc-add-proc-pid-statm-output-validation.patch
>
> This patch will shortly appear at
> https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/selftests-proc-add-proc-pid-statm-output-validation.patch
>
> This patch will later appear in the mm-nonmm-unstable branch at
> git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>
> Before you just go and hit "reply", please:
> a) Consider who else should be cc'ed
> b) Prefer to cc a suitable mailing list as well
> c) Ideally: find the original patch on the mailing list and do a
> reply-to-all to that, adding suitable additional cc's
>
> *** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
>
> The -mm tree is included into linux-next via the mm-everything
> branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> and is updated there every 2-3 working days
>
> ------------------------------------------------------
> From: Swarup Laxman Kotiaklapudi <swarupkotikalapudi@gmail.com>
> Subject: selftests: proc: add /proc/$(pid)/statm output validation
> Date: Wed, 4 Oct 2023 01:13:19 +0530
>
> Add /proc/${pid}/statm validation
>
> /proc/$(pid)/statm output is expected to be:
> "0 0 0 * 0 0 0\n"
> Here * can be any value
>
> Read output of /proc/$(pid)/statm and check except for 4th position, all
> other positions have value zero.
>
> Link: https://lkml.kernel.org/r/20231003194319.602646-1-swarupkotikalapudi@gmail.com
> Signed-off-by: Swarup Laxman Kotiaklapudi <swarupkotikalapudi@gmail.com>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Shuah Khan <shuah@kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
> tools/testing/selftests/proc/proc-empty-vm.c | 57 +++++++++++++++--
> 1 file changed, 52 insertions(+), 5 deletions(-)
>
> --- a/tools/testing/selftests/proc/proc-empty-vm.c~selftests-proc-add-proc-pid-statm-output-validation
> +++ a/tools/testing/selftests/proc/proc-empty-vm.c
> @@ -303,6 +303,56 @@ static int test_proc_pid_smaps_rollup(pi
> }
> }
>
> +static int test_proc_pid_statm(pid_t pid)
> +{
> + char buf[4096];
> + char *tok;
> + char *string;
> + int non_zero_value_indx = 4;
> + int i = 1;
> +
> + snprintf(buf, sizeof(buf), "/proc/%u/statm", pid);
> +
> + /*
> + * Output can be "0 0 0 2 0 0 0\n" where "2" can be anything.
> + */
> + int fd = open(buf, O_RDONLY);
> +
> + if (fd == -1) {
> + if (errno == ENOENT) {
> + /*
> + * /proc/${pid}/statm is under CONFIG_PROC_PAGE_MONITOR,
> + * it doesn't necessarily exist.
> + */
> + return EXIT_SUCCESS;
> + }
> + perror("open /proc/${pid}/statm");
> + return EXIT_FAILURE;
> + } else {
> + ssize_t rv = read(fd, buf, sizeof(buf));
> +
> + close(fd);
> + assert(rv);
> + string = buf;
> +
> + while ((tok = strsep(&string, " ")) != NULL) {
This is unreliable too. read() doesn't terminate the buffer so this relies
on termination from
snprintf(buf, sizeof(buf), "/proc/%u/statm", pid);
Buggy kernel could return a lot of data and overwrite it.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: + selftests-proc-add-proc-pid-statm-output-validation.patch added to mm-nonmm-unstable branch
2023-10-09 6:14 ` + selftests-proc-add-proc-pid-statm-output-validation.patch added to mm-nonmm-unstable branch Alexey Dobriyan
@ 2023-10-09 9:00 ` Alexey Dobriyan
2023-10-09 18:18 ` swarup
2023-10-09 18:16 ` swarup
1 sibling, 1 reply; 7+ messages in thread
From: Alexey Dobriyan @ 2023-10-09 9:00 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, shuah, hughd, swarupkotikalapudi
On Mon, Oct 09, 2023 at 09:14:53AM +0300, Alexey Dobriyan wrote:
> On Wed, Oct 04, 2023 at 01:17:00PM -0700, Andrew Morton wrote:
> > + if (errno == ENOENT) {
> > + /*
> > + * /proc/${pid}/statm is under CONFIG_PROC_PAGE_MONITOR,
> > + * it doesn't necessarily exist.
Oh, and /proc/*/statm is _not_ under CONFIG_PROC_PAGE_MONITOR,
it always exists.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: + selftests-proc-add-proc-pid-statm-output-validation.patch added to mm-nonmm-unstable branch
2023-10-09 6:14 ` + selftests-proc-add-proc-pid-statm-output-validation.patch added to mm-nonmm-unstable branch Alexey Dobriyan
2023-10-09 9:00 ` Alexey Dobriyan
@ 2023-10-09 18:16 ` swarup
1 sibling, 0 replies; 7+ messages in thread
From: swarup @ 2023-10-09 18:16 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: Andrew Morton, linux-kernel, shuah, hughd
eOn Mon, Oct 09, 2023 at 09:14:53AM +0300, Alexey Dobriyan wrote:
> On Wed, Oct 04, 2023 at 01:17:00PM -0700, Andrew Morton wrote:
> >
> > The patch titled
> > Subject: selftests: proc: add /proc/$(pid)/statm output validation
> > has been added to the -mm mm-nonmm-unstable branch. Its filename is
> > selftests-proc-add-proc-pid-statm-output-validation.patch
> >
> > This patch will shortly appear at
> > https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/selftests-proc-add-proc-pid-statm-output-validation.patch
> >
> > This patch will later appear in the mm-nonmm-unstable branch at
> > git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> >
> > Before you just go and hit "reply", please:
> > a) Consider who else should be cc'ed
> > b) Prefer to cc a suitable mailing list as well
> > c) Ideally: find the original patch on the mailing list and do a
> > reply-to-all to that, adding suitable additional cc's
> >
> > *** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
> >
> > The -mm tree is included into linux-next via the mm-everything
> > branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > and is updated there every 2-3 working days
> >
> > ------------------------------------------------------
> > From: Swarup Laxman Kotiaklapudi <swarupkotikalapudi@gmail.com>
> > Subject: selftests: proc: add /proc/$(pid)/statm output validation
> > Date: Wed, 4 Oct 2023 01:13:19 +0530
> >
> > Add /proc/${pid}/statm validation
> >
> > /proc/$(pid)/statm output is expected to be:
> > "0 0 0 * 0 0 0\n"
> > Here * can be any value
> >
> > Read output of /proc/$(pid)/statm and check except for 4th position, all
> > other positions have value zero.
> >
> > Link: https://lkml.kernel.org/r/20231003194319.602646-1-swarupkotikalapudi@gmail.com
> > Signed-off-by: Swarup Laxman Kotiaklapudi <swarupkotikalapudi@gmail.com>
> > Cc: Alexey Dobriyan <adobriyan@gmail.com>
> > Cc: Hugh Dickins <hughd@google.com>
> > Cc: Shuah Khan <shuah@kernel.org>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > ---
> >
> > tools/testing/selftests/proc/proc-empty-vm.c | 57 +++++++++++++++--
> > 1 file changed, 52 insertions(+), 5 deletions(-)
> >
> > --- a/tools/testing/selftests/proc/proc-empty-vm.c~selftests-proc-add-proc-pid-statm-output-validation
> > +++ a/tools/testing/selftests/proc/proc-empty-vm.c
> > @@ -303,6 +303,56 @@ static int test_proc_pid_smaps_rollup(pi
> > }
> > }
> >
> > +static int test_proc_pid_statm(pid_t pid)
> > +{
> > + char buf[4096];
> > + char *tok;
> > + char *string;
> > + int non_zero_value_indx = 4;
> > + int i = 1;
> > +
> > + snprintf(buf, sizeof(buf), "/proc/%u/statm", pid);
> > +
> > + /*
> > + * Output can be "0 0 0 2 0 0 0\n" where "2" can be anything.
> > + */
> > + int fd = open(buf, O_RDONLY);
> > +
> > + if (fd == -1) {
> > + if (errno == ENOENT) {
> > + /*
> > + * /proc/${pid}/statm is under CONFIG_PROC_PAGE_MONITOR,
> > + * it doesn't necessarily exist.
> > + */
> > + return EXIT_SUCCESS;
> > + }
> > + perror("open /proc/${pid}/statm");
> > + return EXIT_FAILURE;
> > + } else {
> > + ssize_t rv = read(fd, buf, sizeof(buf));
> > +
> > + close(fd);
> > + assert(rv);
> > + string = buf;
> > +
> > + while ((tok = strsep(&string, " ")) != NULL) {
>
> This is unreliable too. read() doesn't terminate the buffer so this relies
> on termination from
>
> snprintf(buf, sizeof(buf), "/proc/%u/statm", pid);
>
> Buggy kernel could return a lot of data and overwrite it.
Hi Alexey Dobriyan,
I will try to correct read() function.
Thanks,
Swarup
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: + selftests-proc-add-proc-pid-statm-output-validation.patch added to mm-nonmm-unstable branch
2023-10-09 9:00 ` Alexey Dobriyan
@ 2023-10-09 18:18 ` swarup
0 siblings, 0 replies; 7+ messages in thread
From: swarup @ 2023-10-09 18:18 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: Andrew Morton, linux-kernel, shuah, hughd
esOn Mon, Oct 09, 2023 at 12:00:04PM +0300, Alexey Dobriyan wrote:
> On Mon, Oct 09, 2023 at 09:14:53AM +0300, Alexey Dobriyan wrote:
> > On Wed, Oct 04, 2023 at 01:17:00PM -0700, Andrew Morton wrote:
>
> > > + if (errno == ENOENT) {
> > > + /*
> > > + * /proc/${pid}/statm is under CONFIG_PROC_PAGE_MONITOR,
> > > + * it doesn't necessarily exist.
>
> Oh, and /proc/*/statm is _not_ under CONFIG_PROC_PAGE_MONITOR,
> it always exists.
Hi Alexey Dobriyan,
It is my mistake, i checked the code, yes it always exists.
Thanks,
Swarup
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3] selftests:proc Add /proc/$(pid)/statm output validation
[not found] <20231004201701.87CB5C433C7@smtp.kernel.org>
2023-10-09 6:14 ` + selftests-proc-add-proc-pid-statm-output-validation.patch added to mm-nonmm-unstable branch Alexey Dobriyan
@ 2023-10-09 19:20 ` Swarup Laxman Kotiaklapudi
1 sibling, 0 replies; 7+ messages in thread
From: Swarup Laxman Kotiaklapudi @ 2023-10-09 19:20 UTC (permalink / raw)
To: adobriyan, akpm, linux-kernel, shuah, hughd, mm-commits
Cc: linux-kernel-mentees, Swarup Laxman Kotiaklapudi
Add /proc/${pid}/statm validation
/proc/$(pid)/statm output is expected to be:
"0 0 0 * 0 0 0\n"
Here * can be any value
Read output of /proc/$(pid)/statm
and check except for 4th position,
all other positions have value zero.
Fixes: 5bc73bb3451b (proc: test how it holds up with mapping'less process)
Signed-off-by: Swarup Laxman Kotiaklapudi <swarupkotikalapudi@gmail.com>
---
Changes in V3:
- Remove wrong comment "/proc/${pid}/statm is under
CONFIG_PROC_PAGE_MONITOR"
- read() function usage is change
Changes in V2:
- 4th field if zero it will assert
- field other than 4th field is checked for zero value
- clean up of function test_proc_pid_statm()
tools/testing/selftests/proc/proc-empty-vm.c | 60 ++++++++++++++++++--
1 file changed, 55 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/proc/proc-empty-vm.c b/tools/testing/selftests/proc/proc-empty-vm.c
index b16c13688b88..bc76e8bba7e4 100644
--- a/tools/testing/selftests/proc/proc-empty-vm.c
+++ b/tools/testing/selftests/proc/proc-empty-vm.c
@@ -38,6 +38,8 @@
#include <sys/wait.h>
#include <unistd.h>
+#include "../kselftest.h"
+
#ifdef __amd64__
#define TEST_VSYSCALL
#endif
@@ -302,6 +304,57 @@ static int test_proc_pid_smaps_rollup(pid_t pid)
}
}
+static int test_proc_pid_statm(pid_t pid)
+{
+ char buf[4096];
+ char *tok;
+ char *string, *p;
+ int non_zero_value_indx = 4;
+ int i = 1, len;
+ size_t left;
+
+ snprintf(buf, sizeof(buf), "/proc/%u/statm", pid);
+
+ /*
+ * Output can be "0 0 0 2 0 0 0\n" where "2" can be anything.
+ */
+ int fd = open(buf, O_RDONLY);
+
+ if (fd == -1) {
+ if (errno == ENOENT) {
+ return EXIT_SUCCESS;
+ }
+ perror("open /proc/${pid}/statm");
+ return EXIT_FAILURE;
+ } else {
+ memset(buf, 0, sizeof(buf));
+ left = ARRAY_SIZE(buf);
+ p = buf;
+ while ((len = read(fd, p, left)) > 0) {
+ p += len;
+ left -= len;
+ }
+ close(fd);
+ string = buf;
+
+ while ((tok = strsep(&string, " ")) != NULL) {
+ if (i == non_zero_value_indx) {
+ if (!strncmp(tok, "0", 1))
+ goto err_statm;
+ } else {
+ if (strncmp(tok, "0", 1))
+ goto err_statm;
+ }
+ i++;
+ }
+ }
+
+ return EXIT_SUCCESS;
+
+err_statm:
+ assert(0);
+}
+
int main(void)
{
int rv = EXIT_SUCCESS;
@@ -388,11 +441,8 @@ int main(void)
if (rv == EXIT_SUCCESS) {
rv = test_proc_pid_smaps_rollup(pid);
}
- /*
- * TODO test /proc/${pid}/statm, task_statm()
- * ->start_code, ->end_code aren't updated by munmap().
- * Output can be "0 0 0 2 0 0 0\n" where "2" can be anything.
- */
+ if (rv == EXIT_SUCCESS)
+ rv = test_proc_pid_statm(pid);
/* Cut the rope. */
int wstatus;
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: + selftests-proc-add-proc-pid-statm-output-validation.patch added to mm-nonmm-unstable branch
2023-10-02 12:38 ` + selftests-proc-add-proc-pid-statm-output-validation.patch added to mm-nonmm-unstable branch Alexey Dobriyan
@ 2023-10-02 17:52 ` swarup
0 siblings, 0 replies; 7+ messages in thread
From: swarup @ 2023-10-02 17:52 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: Andrew Morton, linux-kernel, shuah, hughd
On Mon, Oct 02, 2023 at 03:38:25PM +0300, Alexey Dobriyan wrote:
> On Sun, Oct 01, 2023 at 12:37:40PM -0700, Andrew Morton wrote:
> > selftests-proc-add-proc-pid-statm-output-validation.patch
>
> > Add /proc/${pid}/statm validation
> >
> > /proc/$(pid)/statm output is expected to be:
> > "0 0 0 * 0 0 0\n"
> > Here * can be any value
> >
> > Read output of /proc/$(pid)/statm
> > and compare length of output is
> > equal or greater than expected output
>
> > --- a/tools/testing/selftests/proc/proc-empty-vm.c~selftests-proc-add-proc-pid-statm-output-validation
> > +++ a/tools/testing/selftests/proc/proc-empty-vm.c
> > @@ -303,6 +303,37 @@ static int test_proc_pid_smaps_rollup(pi
> > }
> > }
> >
> > +static const char g_statm[] = "0 0 0 * 0 0 0\n";
>
> This is both unreliable and incorrect.
>
> 4th value is "end_code - start_code" when exec is done which could be
> anything not 1-digit number (although unlikely).
>
> Testing for strlen is simply too weak of a test.
>
> > +static int test_proc_pid_statm(pid_t pid)
> > +{
> > + char buf[4096];
> > +
> > + snprintf(buf, sizeof(buf), "/proc/%u/statm", pid);
> > +
> > + int fd = open(buf, O_RDONLY);
> > +
> > + if (fd == -1) {
> > + if (errno == ENOENT) {
> > + /*
> > + * /proc/${pid}/statm is under CONFIG_PROC_PAGE_MONITOR,
> > + * it doesn't necessarily exist.
> > + */
> > + return EXIT_SUCCESS;
> > + }
> > + perror("open /proc/${pid}/statm");
> > + return EXIT_FAILURE;
> > + } else {
> > + ssize_t rv = read(fd, buf, sizeof(buf));
> > +
> > + close(fd);
> > + size_t len = strlen(g_statm);
> > +
> > + assert(rv >= len);
> > + return EXIT_SUCCESS;
> > + }
> > +}
> > +
> > int main(void)
> > {
> > int rv = EXIT_SUCCESS;
> > @@ -389,11 +420,8 @@ int main(void)
> > if (rv == EXIT_SUCCESS) {
> > rv = test_proc_pid_smaps_rollup(pid);
> > }
> > - /*
> > - * TODO test /proc/${pid}/statm, task_statm()
> > - * ->start_code, ->end_code aren't updated by munmap().
> > - * Output can be "0 0 0 2 0 0 0\n" where "2" can be anything.
> > - */
> > + if (rv == EXIT_SUCCESS)
> > + rv = test_proc_pid_statm(pid);
> >
> > /* Cut the rope. */
Hi Alexey,
Thanks for reviewing the changes.
I assume below output of /proc/${procid}/statm
can be assumed as mentioned below:
static const char g_statm[] = "0 0 0 * 0 0 0\n"
If 0 is correct at their places, only issue is *,
whose value will be single digit or could change?
If this assumption is correct, i can change the
validation to handle 4th postion, and remaining
place will validate if it has zero or not,
and will send another patch?
Thanks,
Swarup
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: + selftests-proc-add-proc-pid-statm-output-validation.patch added to mm-nonmm-unstable branch
[not found] <20231001193740.B716AC433C7@smtp.kernel.org>
@ 2023-10-02 12:38 ` Alexey Dobriyan
2023-10-02 17:52 ` swarup
0 siblings, 1 reply; 7+ messages in thread
From: Alexey Dobriyan @ 2023-10-02 12:38 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, shuah, hughd, swarupkotikalapudi
On Sun, Oct 01, 2023 at 12:37:40PM -0700, Andrew Morton wrote:
> selftests-proc-add-proc-pid-statm-output-validation.patch
> Add /proc/${pid}/statm validation
>
> /proc/$(pid)/statm output is expected to be:
> "0 0 0 * 0 0 0\n"
> Here * can be any value
>
> Read output of /proc/$(pid)/statm
> and compare length of output is
> equal or greater than expected output
> --- a/tools/testing/selftests/proc/proc-empty-vm.c~selftests-proc-add-proc-pid-statm-output-validation
> +++ a/tools/testing/selftests/proc/proc-empty-vm.c
> @@ -303,6 +303,37 @@ static int test_proc_pid_smaps_rollup(pi
> }
> }
>
> +static const char g_statm[] = "0 0 0 * 0 0 0\n";
This is both unreliable and incorrect.
4th value is "end_code - start_code" when exec is done which could be
anything not 1-digit number (although unlikely).
Testing for strlen is simply too weak of a test.
> +static int test_proc_pid_statm(pid_t pid)
> +{
> + char buf[4096];
> +
> + snprintf(buf, sizeof(buf), "/proc/%u/statm", pid);
> +
> + int fd = open(buf, O_RDONLY);
> +
> + if (fd == -1) {
> + if (errno == ENOENT) {
> + /*
> + * /proc/${pid}/statm is under CONFIG_PROC_PAGE_MONITOR,
> + * it doesn't necessarily exist.
> + */
> + return EXIT_SUCCESS;
> + }
> + perror("open /proc/${pid}/statm");
> + return EXIT_FAILURE;
> + } else {
> + ssize_t rv = read(fd, buf, sizeof(buf));
> +
> + close(fd);
> + size_t len = strlen(g_statm);
> +
> + assert(rv >= len);
> + return EXIT_SUCCESS;
> + }
> +}
> +
> int main(void)
> {
> int rv = EXIT_SUCCESS;
> @@ -389,11 +420,8 @@ int main(void)
> if (rv == EXIT_SUCCESS) {
> rv = test_proc_pid_smaps_rollup(pid);
> }
> - /*
> - * TODO test /proc/${pid}/statm, task_statm()
> - * ->start_code, ->end_code aren't updated by munmap().
> - * Output can be "0 0 0 2 0 0 0\n" where "2" can be anything.
> - */
> + if (rv == EXIT_SUCCESS)
> + rv = test_proc_pid_statm(pid);
>
> /* Cut the rope. */
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-10-09 19:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20231004201701.87CB5C433C7@smtp.kernel.org>
2023-10-09 6:14 ` + selftests-proc-add-proc-pid-statm-output-validation.patch added to mm-nonmm-unstable branch Alexey Dobriyan
2023-10-09 9:00 ` Alexey Dobriyan
2023-10-09 18:18 ` swarup
2023-10-09 18:16 ` swarup
2023-10-09 19:20 ` [PATCH v3] selftests:proc Add /proc/$(pid)/statm output validation Swarup Laxman Kotiaklapudi
[not found] <20231001193740.B716AC433C7@smtp.kernel.org>
2023-10-02 12:38 ` + selftests-proc-add-proc-pid-statm-output-validation.patch added to mm-nonmm-unstable branch Alexey Dobriyan
2023-10-02 17:52 ` swarup
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).