[LTP] [PATCH v4 2/4] syscalls/madvise11: new test for madvise(MADV_DONTNEED)

[zhaogongyi]

Hi,

> 
> Hello,
> 
> Zhao Gongyi via ltp <ltp@lists.linux.it> writes:
> 
> > Check that madvise(2) MADV_DONTNEED applied to shared mappings
> will
> > lead to the resident set size(RSS) of the calling process reduced
> immediately.
> >
> > Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
> > ---
> >  runtest/syscalls                              |  1 +
> >  testcases/kernel/syscalls/madvise/.gitignore  |  1 +
> > testcases/kernel/syscalls/madvise/madvise11.c | 82
> +++++++++++++++++++
> >  3 files changed, 84 insertions(+)
> >  create mode 100644 testcases/kernel/syscalls/madvise/madvise11.c
> >
> > diff --git a/runtest/syscalls b/runtest/syscalls index
> > eb1910cec..296af9f9d 100644
> > --- a/runtest/syscalls
> > +++ b/runtest/syscalls
> > @@ -948,6 +948,7 @@ madvise07 madvise07
> >  madvise08 madvise08
> >  madvise09 madvise09
> >  madvise10 madvise10
> > +madvise11 madvise11
> >
> >  newuname01 newuname01
> >
> > diff --git a/testcases/kernel/syscalls/madvise/.gitignore
> > b/testcases/kernel/syscalls/madvise/.gitignore
> > index db8ce47c1..ffd8823d1 100644
> > --- a/testcases/kernel/syscalls/madvise/.gitignore
> > +++ b/testcases/kernel/syscalls/madvise/.gitignore
> > @@ -8,3 +8,4 @@
> >  /madvise08
> >  /madvise09
> >  /madvise10
> > +/madvise11
> > diff --git a/testcases/kernel/syscalls/madvise/madvise11.c
> > b/testcases/kernel/syscalls/madvise/madvise11.c
> > new file mode 100644
> > index 000000000..0132c091c
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/madvise/madvise11.c
> > @@ -0,0 +1,82 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) Huawei Technologies Co., Ltd. 2022. All rights reserved.
> > + * Author: Zhao Gongyi <zhaogongyi@huawei.com>  */
> > +
> > +/*\
> > + * [Description]
> > + *
> > + * Check that madvise(2) MADV_DONTNEED applied to shared
> mappings
> > +will lead to
> > + * the resident set size(RSS) of the calling process reduced immediately.
> > + */
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include "tst_test.h"
> > +
> > +#define MAP_SIZE (8 * 1024)
> > +#define BUF_SIZE 1024
> > +
> > +static FILE *fp;
> > +static char *addr;
> > +
> > +static void run(void)
> > +{
> > +	char cmd[BUF_SIZE];
> > +	char line[BUF_SIZE];
> > +	char vm_area_addr[128];
> > +
> > +	TEST(madvise(addr, MAP_SIZE, MADV_DONTNEED));
> > +	if (TST_RET == -1) {
> > +		tst_brk(TBROK | TTERRNO, "madvise(%p, %d, 0x%x) failed",
> > +			addr, MAP_SIZE, MADV_DONTNEED);
> > +	}
> 
> You have a lot of madvise patches, so why not create SAFE_MADVISE?

Yes, it seems more better, thanks!


> 
> Or there are the TST_EXP_* macros.
> 
> > +
> > +	sprintf(vm_area_addr, "%p", addr);
> > +	sprintf(cmd, "cat /proc/%d/smaps", getpid());
> > +	fp = popen(cmd, "r");
> > +
> > +	/* Find the vm area */
> > +	while (fgets(line, sizeof(line), fp) != NULL) {
> > +		if (strstr(line, &(vm_area_addr[2])))
> 
> AFAICT this could match more than one line by matching the end of the
> preceding range.

Is it only one vma that inlucde the address?

> 
> I think that in general it's better to avoid strstr if memcmp can be easily
> used instead.
> 
> > +			break;
> > +	}
> > +
> > +	/* Find Rss size of the vm area */
> > +	while (fgets(line, sizeof(line), fp) != NULL) {
> > +		if (strstr(line, "Rss:")) {
> 
> Same here although Rss: seems to be unique. Then again it could be added
> to another field in new kernel.
> 
> Perhaps sscanf would be better?

Maybe it will encounter the same problem with sscanf when the field changed?

It seems there is only one way to get the RSS.

Regards,
Gongyi