* [LTP] [PATCH v2] mem/hugetlb: shift an empty region to use in hugemmap02.c
@ 2015-10-12 10:53 Li Wang
2015-10-12 18:29 ` Cyril Hrubis
0 siblings, 1 reply; 8+ messages in thread
From: Li Wang @ 2015-10-12 10:53 UTC (permalink / raw)
To: ltp
v1 --> v2
1. add a function 'range_is_mapped()' in the mem.c
2. choice the mmap address region dynamically
the testcase 'hugemmap02' always failed on s390x as:
----------
hugemmap02 0 TINFO : set nr_hugepages to 128
hugemmap02 1 TPASS : huge mmap succeeded (64-bit)
./hugemmap02: relocation error: ./hugemmap02: symbol , version GLIBC_2.2 not defined in file libc.so.6 with link time reference
----------
it is caused by mapping in zero where libc.so.6 was loaded and resulting in
the dynamic loader finding no information about versions for the library.
running the program, we could obviously see the region area:
---------------
program address:
80000000-80001000 r--s 00000000 00:05 2644 /dev/zero
80001000-80012000 r-xp 00001000 fd:00 35121302 /mnt/tests/kernel/distribution/ltp/generic/ltp-full-20150903/testcases/kernel/mem/hugetlb/hugemmap/hugemmap02
80012000-80013000 r--p 00011000 fd:00 35121302 /mnt/tests/kernel/distribution/ltp/generic/ltp-full-20150903/testcases/kernel/mem/hugetlb/hugemmap/hugemmap02
80013000-80014000 rw-p 00012000 fd:00 35121302 /mnt/tests/kernel/distribution/ltp/generic/ltp-full-20150903/testcases/kernel/mem/hugetlb/hugemmap/hugemmap02
80014000-80017000 rw-p 00000000 00:00 0
90000000-90200000 rw-s 00000000 00:29 75950 /tmp/hugme85Xf/mmapfile41071
9c14f000-9c170000 rw-p 00000000 00:00 0 [heap]
3ffad342000-3ffbd342000 r--s 00000000 00:05 2644 /dev/zero
3ffbd342000-3ffcd342000 r--s 00000000 00:05 2644 /dev/zero
3ffcd342000-3ffdd342000 r--s 00000000 00:05 2644 /dev/zero
3ffdd342000-3ffed342000 r--s 00000000 00:05 2644 /dev/zero
3ffed342000-3fffd342000 r--s 00000000 00:05 2644 /dev/zero
3fffd342000-3fffd344000 rw-p 00000000 00:00 0
3fffd344000-3fffd4db000 r-xp 00000000 fd:00 5438 /usr/lib64/libc-2.17.so
3fffd4db000-3fffd4df000 r--p 00196000 fd:00 5438 /usr/lib64/libc-2.17.so
3fffd4df000-3fffd4e1000 rw-p 0019a000 fd:00 5438 /usr/lib64/libc-2.17.so
3fffd4e1000-3fffd4e6000 rw-p 00000000 00:00 0
3fffd4e6000-3fffd4fe000 r-xp 00000000 fd:00 5464 /usr/lib64/libpthread-2.17.so
3fffd4fe000-3fffd4ff000 r--p 00017000 fd:00 5464 /usr/lib64/libpthread-2.17.so
3fffd4ff000-3fffd500000 rw-p 00018000 fd:00 5464 /usr/lib64/libpthread-2.17.so
3fffd500000-3fffd504000 rw-p 00000000 00:00 0
3fffd50c000-3fffd510000 rw-p 00000000 00:00 0
3fffd510000-3fffd512000 r-xp 00000000 00:00 0 [vdso]
3fffd512000-3fffd534000 r-xp 00000000 fd:00 5431 /usr/lib64/ld-2.17.so
3fffd534000-3fffd535000 r--p 00022000 fd:00 5431 /usr/lib64/ld-2.17.so
3fffd535000-3fffd536000 rw-p 00023000 fd:00 5431 /usr/lib64/ld-2.17.so
3fffd536000-3fffd537000 rw-p 00000000 00:00 0
3ffffaaf000-3ffffad0000 rw-p 00000000 00:00 0 [stack]
-------------
Signed-off-by: Li Wang <liwang@redhat.com>
---
testcases/kernel/mem/hugetlb/hugemmap/hugemmap02.c | 24 ++++++++++--
testcases/kernel/mem/include/mem.h | 2 +
testcases/kernel/mem/lib/mem.c | 43 ++++++++++++++++++++++
3 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap02.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap02.c
index cfdcb3c..a4fbff9 100644
--- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap02.c
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap02.c
@@ -56,8 +56,8 @@
#include "safe_macros.h"
#include "mem.h"
-#define LOW_ADDR (void *)(0x80000000)
-#define LOW_ADDR2 (void *)(0x90000000)
+#define LOW_ADDR 0x80000000
+#define LOW_ADDR2 0x90000000
static char TEMPFILE[MAXPATHLEN];
@@ -65,6 +65,8 @@ char *TCID = "hugemmap02";
int TST_TOTAL = 1;
static unsigned long *addr;
static unsigned long *addr2;
+static unsigned long low_addr = LOW_ADDR;
+static unsigned long low_addr2 = LOW_ADDR2;
static unsigned long *addrlist[5];
static int i;
static int fildes;
@@ -127,15 +129,29 @@ int main(int ac, char **av)
addrlist[i] = addr;
}
+ while (range_is_mapped(low_addr, low_addr + 0x1000) == 1) {
+ low_addr = low_addr + 0x10000000;
+
+ if (low_addr < LOW_ADDR)
+ tst_brkm(TBROK | TERRNO, cleanup,
+ "no empty region to use");
+ }
/* mmap using normal pages and a low memory address */
- addr = mmap(LOW_ADDR, page_sz, PROT_READ,
+ addr = mmap((void *)low_addr, page_sz, PROT_READ,
MAP_SHARED | MAP_FIXED, nfildes, 0);
if (addr == MAP_FAILED)
tst_brkm(TBROK | TERRNO, cleanup,
"mmap failed on nfildes");
+ while (range_is_mapped(low_addr2, low_addr2 + 0x1000) == 1) {
+ low_addr2 = low_addr2 + 0x10000000;
+
+ if (low_addr2 < LOW_ADDR2)
+ tst_brkm(TBROK | TERRNO, cleanup,
+ "no empty region to use");
+ }
/* Attempt to mmap a huge page into a low memory address */
- addr2 = mmap(LOW_ADDR2, map_sz, PROT_READ | PROT_WRITE,
+ addr2 = mmap((void *)low_addr2, map_sz, PROT_READ | PROT_WRITE,
MAP_SHARED, fildes, 0);
#if __WORDSIZE == 64 /* 64-bit process */
if (addr2 == MAP_FAILED) {
diff --git a/testcases/kernel/mem/include/mem.h b/testcases/kernel/mem/include/mem.h
index fd1879c..7ed3835 100644
--- a/testcases/kernel/mem/include/mem.h
+++ b/testcases/kernel/mem/include/mem.h
@@ -100,4 +100,6 @@ void setup(void);
void update_shm_size(size_t *shm_size);
+/* MMAP */
+int range_is_mapped(unsigned long low, unsigned long high)
#endif
diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index 118b6c0..e0e1bdf 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -1113,3 +1113,46 @@ void update_shm_size(size_t * shm_size)
*shm_size = shmmax;
}
}
+
+int range_is_mapped(unsigned long low, unsigned long high)
+{
+ FILE *f;
+ int MAPS_BUF_SZ = 4096;
+ char line[MAPS_BUF_SZ];
+ char *tmp;
+
+ f = fopen("/proc/self/maps", "r");
+ if (!f) {
+ printf("Failed to open /proc/self/maps.\n");
+ return -1;
+ }
+
+ while (1) {
+ unsigned long start, end;
+ int ret;
+
+ tmp = fgets(line, MAPS_BUF_SZ, f);
+ if (!tmp)
+ break;
+
+ ret = sscanf(line, "%lx-%lx", &start, &end);
+ if (ret != 2) {
+ printf("Couldn't parse /proc/self/maps line: %s\n",
+ line);
+ fclose(f);
+ return -1;
+ }
+
+ if ((start >= low) && (start < high)) {
+ fclose(f);
+ return 1;
+ }
+ if ((end >= low) && (end < high)) {
+ fclose(f);
+ return 1;
+ }
+ }
+
+ fclose(f);
+ return 0;
+}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH v2] mem/hugetlb: shift an empty region to use in hugemmap02.c
2015-10-12 10:53 [LTP] [PATCH v2] mem/hugetlb: shift an empty region to use in hugemmap02.c Li Wang
@ 2015-10-12 18:29 ` Cyril Hrubis
2015-10-13 5:39 ` Li Wang
0 siblings, 1 reply; 8+ messages in thread
From: Cyril Hrubis @ 2015-10-12 18:29 UTC (permalink / raw)
To: ltp
Hi!
Generall idea looks good, a few comments below.
> +/* MMAP */
> +int range_is_mapped(unsigned long low, unsigned long high)
> #endif
> diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
> index 118b6c0..e0e1bdf 100644
> --- a/testcases/kernel/mem/lib/mem.c
> +++ b/testcases/kernel/mem/lib/mem.c
> @@ -1113,3 +1113,46 @@ void update_shm_size(size_t * shm_size)
> *shm_size = shmmax;
> }
> }
> +
> +int range_is_mapped(unsigned long low, unsigned long high)
> +{
> + FILE *f;
> + int MAPS_BUF_SZ = 4096;
> + char line[MAPS_BUF_SZ];
> + char *tmp;
> +
> + f = fopen("/proc/self/maps", "r");
> + if (!f) {
> + printf("Failed to open /proc/self/maps.\n");
^
should rather be tst_resm(TWARN, );
> + return -1;
> + }
> +
> + while (1) {
> + unsigned long start, end;
> + int ret;
> +
> + tmp = fgets(line, MAPS_BUF_SZ, f);
> + if (!tmp)
> + break;
> +
> + ret = sscanf(line, "%lx-%lx", &start, &end);
> + if (ret != 2) {
> + printf("Couldn't parse /proc/self/maps line: %s\n",
> + line);
^
here as well
> + fclose(f);
> + return -1;
> + }
Why don't you just use fscanf()? It doesn't make sense to read the line
into the buffer and then sscanf() the values.
> + if ((start >= low) && (start < high)) {
> + fclose(f);
> + return 1;
> + }
> + if ((end >= low) && (end < high)) {
> + fclose(f);
> + return 1;
> + }
> + }
> +
> + fclose(f);
> + return 0;
> +}
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v2] mem/hugetlb: shift an empty region to use in hugemmap02.c
2015-10-12 18:29 ` Cyril Hrubis
@ 2015-10-13 5:39 ` Li Wang
2015-10-13 10:15 ` Li Wang
0 siblings, 1 reply; 8+ messages in thread
From: Li Wang @ 2015-10-13 5:39 UTC (permalink / raw)
To: ltp
Hi,
On Tue, Oct 13, 2015 at 2:29 AM, Cyril Hrubis <chrubis@suse.cz> wrote:
> Hi!
> Generall idea looks good, a few comments below.
>
> > +/* MMAP */
> > +int range_is_mapped(unsigned long low, unsigned long high)
> > #endif
> > diff --git a/testcases/kernel/mem/lib/mem.c
> b/testcases/kernel/mem/lib/mem.c
> > index 118b6c0..e0e1bdf 100644
> > --- a/testcases/kernel/mem/lib/mem.c
> > +++ b/testcases/kernel/mem/lib/mem.c
> > @@ -1113,3 +1113,46 @@ void update_shm_size(size_t * shm_size)
> > *shm_size = shmmax;
> > }
> > }
> > +
> > +int range_is_mapped(unsigned long low, unsigned long high)
> > +{
> > + FILE *f;
> > + int MAPS_BUF_SZ = 4096;
> > + char line[MAPS_BUF_SZ];
> > + char *tmp;
> > +
> > + f = fopen("/proc/self/maps", "r");
> > + if (!f) {
> > + printf("Failed to open /proc/self/maps.\n");
> ^
> should rather be tst_resm(TWARN, );
>
ok. looks like using
FILE * f = SAFE_FOPEN(NULL, "/proc/self/maps", "r");
is better than above.
> > + return -1;
> > + }
> > +
> > + while (1) {
> > + unsigned long start, end;
> > + int ret;
> > +
> > + tmp = fgets(line, MAPS_BUF_SZ, f);
> > + if (!tmp)
> > + break;
> > +
> > + ret = sscanf(line, "%lx-%lx", &start, &end);
> > + if (ret != 2) {
> > + printf("Couldn't parse /proc/self/maps line: %s\n",
> > + line);
> ^
> here as well
>
ok.
> > + fclose(f);
> > + return -1;
> > + }
>
> Why don't you just use fscanf()? It doesn't make sense to read the line
> into the buffer and then sscanf() the values.
>
hmm, we need to read the '/proc/self/maps' line by line, It easy to use fgets()
to get each line in and then sscanf() that.
fscanf() will never read the newline at the end of the first line. So when
it is called the second time, it will fail (returning 0, not EOF) and read
nothing, leaving the [start, end] unchanged.
maybe I don't know if fscanf() could achieve that also?
so what about something like:
--------------------------------------
int range_is_mapped(unsigned long low, unsigned long high)
{
FILE *f = SAFE_FOPEN(NULL, "/proc/self/maps", "r");
int MAPS_BUF_SZ = 4096;
char line[MAPS_BUF_SZ];
while (fgets(line, MAPS_BUF_SZ, f)) {
unsigned long start, end;
int ret;
ret = sscanf(line, "%lx-%lx", &start, &end);
if (ret != 2) {
tst_resm(TWARN | TERRNO, "Couldn't parse /proc/self/maps
line.");
SAFE_FCLOSE(NULL, f);
}
if ((start >= low) && (start < high)) {
SAFE_FCLOSE(NULL, f);
return 1;
}
if ((end >= low) && (end < high)) {
SAFE_FCLOSE(NULL, f);
return 1;
}
}
SAFE_FCLOSE(NULL, f);
return 0;
}
> > + if ((start >= low) && (start < high)) {
> > + fclose(f);
> > + return 1;
> > + }
> > + if ((end >= low) && (end < high)) {
> > + fclose(f);
> > + return 1;
> > + }
> > + }
> > +
> > + fclose(f);
> > + return 0;
> > +}
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
--
Regards,
Li Wang
Email: liwang@redhat.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20151013/95af04a5/attachment-0001.html>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v2] mem/hugetlb: shift an empty region to use in hugemmap02.c
2015-10-13 5:39 ` Li Wang
@ 2015-10-13 10:15 ` Li Wang
2015-10-13 10:53 ` Cyril Hrubis
2015-10-13 10:54 ` Jan Stancek
0 siblings, 2 replies; 8+ messages in thread
From: Li Wang @ 2015-10-13 10:15 UTC (permalink / raw)
To: ltp
Hi,
On Tue, Oct 13, 2015 at 1:39 PM, Li Wang <liwang@redhat.com> wrote:
> Hi,
>
> On Tue, Oct 13, 2015 at 2:29 AM, Cyril Hrubis <chrubis@suse.cz> wrote:
>
>> Hi!
>> Generall idea looks good, a few comments below.
>>
>> > +/* MMAP */
>> > +int range_is_mapped(unsigned long low, unsigned long high)
>> > #endif
>> > diff --git a/testcases/kernel/mem/lib/mem.c
>> b/testcases/kernel/mem/lib/mem.c
>> > index 118b6c0..e0e1bdf 100644
>> > --- a/testcases/kernel/mem/lib/mem.c
>> > +++ b/testcases/kernel/mem/lib/mem.c
>> > @@ -1113,3 +1113,46 @@ void update_shm_size(size_t * shm_size)
>> > *shm_size = shmmax;
>> > }
>> > }
>> > +
>> > +int range_is_mapped(unsigned long low, unsigned long high)
>> > +{
>> > + FILE *f;
>> > + int MAPS_BUF_SZ = 4096;
>> > + char line[MAPS_BUF_SZ];
>> > + char *tmp;
>> > +
>> > + f = fopen("/proc/self/maps", "r");
>> > + if (!f) {
>> > + printf("Failed to open /proc/self/maps.\n");
>> ^
>> should rather be tst_resm(TWARN, );
>>
>
> ok. looks like using
> FILE * f = SAFE_FOPEN(NULL, "/proc/self/maps", "r");
> is better than above.
>
>
>> > + return -1;
>> > + }
>> > +
>> > + while (1) {
>> > + unsigned long start, end;
>> > + int ret;
>> > +
>> > + tmp = fgets(line, MAPS_BUF_SZ, f);
>> > + if (!tmp)
>> > + break;
>> > +
>> > + ret = sscanf(line, "%lx-%lx", &start, &end);
>> > + if (ret != 2) {
>> > + printf("Couldn't parse /proc/self/maps line:
>> %s\n",
>> > + line);
>> ^
>> here as well
>>
>
> ok.
>
>
>> > + fclose(f);
>> > + return -1;
>> > + }
>>
>> Why don't you just use fscanf()? It doesn't make sense to read the line
>> into the buffer and then sscanf() the values.
>>
>
okay, seems you are right. I tried and achieved another one, following
code test pass on my s390x system.
-----------------------------------------
int range_is_mapped(unsigned long low, unsigned long high)
{
FILE *f = SAFE_FOPEN(NULL, "/proc/self/maps", "r");
char buf[512];
while (!feof(f)) {
unsigned long start, end;
int ret;
ret = fscanf(f, "%lx-%lx %[^\n]%*c]", &start, &end, buf);
if (ret != 3) {
tst_resm(TWARN | TERRNO, "Couldn't parse /proc/self/maps
line.");
SAFE_FCLOSE(NULL, f);
}
if ((start >= low) && (start < high)) {
SAFE_FCLOSE(NULL, f);
return 1;
}
if ((end >= low) && (end < high)) {
SAFE_FCLOSE(NULL, f);
return 1;
}
}
SAFE_FCLOSE(NULL, f);
return 0;
}
so, which one is better? I will format patch V3 when getting some
proposals.
cc' jstancek@ if has some good idea.
> hmm, we need to read the '/proc/self/maps' line by line, It easy to use fgets()
> to get each line in and then sscanf() that.
>
> fscanf() will never read the newline at the end of the first line. So
> when it is called the second time, it will fail (returning 0, not EOF) and
> read nothing, leaving the [start, end] unchanged.
> maybe I don't know if fscanf() could achieve that also?
>
> so what about something like:
> --------------------------------------
> int range_is_mapped(unsigned long low, unsigned long high)
> {
> FILE *f = SAFE_FOPEN(NULL, "/proc/self/maps", "r");
>
> int MAPS_BUF_SZ = 4096;
> char line[MAPS_BUF_SZ];
>
> while (fgets(line, MAPS_BUF_SZ, f)) {
> unsigned long start, end;
> int ret;
>
> ret = sscanf(line, "%lx-%lx", &start, &end);
> if (ret != 2) {
> tst_resm(TWARN | TERRNO, "Couldn't parse /proc/self/maps
> line.");
> SAFE_FCLOSE(NULL, f);
> }
>
> if ((start >= low) && (start < high)) {
> SAFE_FCLOSE(NULL, f);
> return 1;
> }
> if ((end >= low) && (end < high)) {
> SAFE_FCLOSE(NULL, f);
> return 1;
> }
> }
>
> SAFE_FCLOSE(NULL, f);
> return 0;
> }
>
>
>
>> > + if ((start >= low) && (start < high)) {
>> > + fclose(f);
>> > + return 1;
>> > + }
>> > + if ((end >= low) && (end < high)) {
>> > + fclose(f);
>> > + return 1;
>> > + }
>> > + }
>> > +
>> > + fclose(f);
>> > + return 0;
>> > +}
>>
>> --
>> Cyril Hrubis
>> chrubis@suse.cz
>>
>
>
>
> --
> Regards,
> Li Wang
> Email: liwang@redhat.com
>
--
Regards,
Li Wang
Email: liwang@redhat.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20151013/6ec80a51/attachment.html>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v2] mem/hugetlb: shift an empty region to use in hugemmap02.c
2015-10-13 10:15 ` Li Wang
@ 2015-10-13 10:53 ` Cyril Hrubis
2015-10-13 13:39 ` Li Wang
2015-10-13 10:54 ` Jan Stancek
1 sibling, 1 reply; 8+ messages in thread
From: Cyril Hrubis @ 2015-10-13 10:53 UTC (permalink / raw)
To: ltp
Hi!
> okay, seems you are right. I tried and achieved another one, following
> code test pass on my s390x system.
> -----------------------------------------
> int range_is_mapped(unsigned long low, unsigned long high)
> {
> FILE *f = SAFE_FOPEN(NULL, "/proc/self/maps", "r");
If we start to use SAFE_MACROS() here we have to pass the cleanup
callback to the range_is_mapped() as a first parameter so that we can
pass correct cleanup callback to the SAFE_MACROS().
> char buf[512];
>
> while (!feof(f)) {
> unsigned long start, end;
> int ret;
>
> ret = fscanf(f, "%lx-%lx %[^\n]%*c]", &start, &end, buf);
You can do even better with:
ret = fscanf(f, "%lx-%lx %*[^\n]\n", &start, &end);
Which says read two long ints and then ignore everything until the end
of line.
You could have ommited the last '\n' in the format string, but then you
would need to exit the loop when the fscanf() returns -1 since the
feof() would not exit the loop since there would still be the last
newline in the buffer after we read the last line...
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v2] mem/hugetlb: shift an empty region to use in hugemmap02.c
2015-10-13 10:15 ` Li Wang
2015-10-13 10:53 ` Cyril Hrubis
@ 2015-10-13 10:54 ` Jan Stancek
2015-10-13 11:11 ` Jan Stancek
1 sibling, 1 reply; 8+ messages in thread
From: Jan Stancek @ 2015-10-13 10:54 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Li Wang" <liwang@redhat.com>
> To: "Cyril Hrubis" <chrubis@suse.cz>, "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Tuesday, 13 October, 2015 12:15:23 PM
> Subject: Re: [LTP] [PATCH v2] mem/hugetlb: shift an empty region to use in hugemmap02.c
>
> Hi,
>
> On Tue, Oct 13, 2015 at 1:39 PM, Li Wang <liwang@redhat.com> wrote:
>
> > Hi,
> >
> > On Tue, Oct 13, 2015 at 2:29 AM, Cyril Hrubis <chrubis@suse.cz> wrote:
> >
> >> Hi!
> >> Generall idea looks good, a few comments below.
> >>
> >> > +/* MMAP */
> >> > +int range_is_mapped(unsigned long low, unsigned long high)
> >> > #endif
> >> > diff --git a/testcases/kernel/mem/lib/mem.c
> >> b/testcases/kernel/mem/lib/mem.c
> >> > index 118b6c0..e0e1bdf 100644
> >> > --- a/testcases/kernel/mem/lib/mem.c
> >> > +++ b/testcases/kernel/mem/lib/mem.c
> >> > @@ -1113,3 +1113,46 @@ void update_shm_size(size_t * shm_size)
> >> > *shm_size = shmmax;
> >> > }
> >> > }
> >> > +
> >> > +int range_is_mapped(unsigned long low, unsigned long high)
> >> > +{
> >> > + FILE *f;
> >> > + int MAPS_BUF_SZ = 4096;
> >> > + char line[MAPS_BUF_SZ];
> >> > + char *tmp;
> >> > +
> >> > + f = fopen("/proc/self/maps", "r");
> >> > + if (!f) {
> >> > + printf("Failed to open /proc/self/maps.\n");
> >> ^
> >> should rather be tst_resm(TWARN, );
> >>
> >
> > ok. looks like using
> > FILE * f = SAFE_FOPEN(NULL, "/proc/self/maps", "r");
> > is better than above.
> >
> >
> >> > + return -1;
> >> > + }
> >> > +
> >> > + while (1) {
> >> > + unsigned long start, end;
> >> > + int ret;
> >> > +
> >> > + tmp = fgets(line, MAPS_BUF_SZ, f);
> >> > + if (!tmp)
> >> > + break;
> >> > +
> >> > + ret = sscanf(line, "%lx-%lx", &start, &end);
> >> > + if (ret != 2) {
> >> > + printf("Couldn't parse /proc/self/maps line:
> >> %s\n",
> >> > + line);
> >> ^
> >> here as well
> >>
> >
> > ok.
> >
> >
> >> > + fclose(f);
> >> > + return -1;
> >> > + }
> >>
> >> Why don't you just use fscanf()? It doesn't make sense to read the line
> >> into the buffer and then sscanf() the values.
> >>
> >
> okay, seems you are right. I tried and achieved another one, following
> code test pass on my s390x system.
> -----------------------------------------
> int range_is_mapped(unsigned long low, unsigned long high)
> {
> FILE *f = SAFE_FOPEN(NULL, "/proc/self/maps", "r");
>
> char buf[512];
>
> while (!feof(f)) {
> unsigned long start, end;
> int ret;
>
> ret = fscanf(f, "%lx-%lx %[^\n]%*c]", &start, &end, buf);
> if (ret != 3) {
> tst_resm(TWARN | TERRNO, "Couldn't parse /proc/self/maps
> line.");
> SAFE_FCLOSE(NULL, f);
> }
>
> if ((start >= low) && (start < high)) {
> SAFE_FCLOSE(NULL, f);
> return 1;
> }
> if ((end >= low) && (end < high)) {
> SAFE_FCLOSE(NULL, f);
> return 1;
> }
> }
>
> SAFE_FCLOSE(NULL, f);
> return 0;
> }
>
> so, which one is better? I will format patch V3 when getting some
> proposals.
>
> cc' jstancek@ if has some good idea.
>
The approach above should work, but it also stores data to buf,
that we don't care about. You could do fscanf + "while (fgetc(f) != '\n');"
to forward to next line, if you want to avoid using buf.
Anyway, I'd go with fgets() + sscanf(), it wastes couple bytes on stack,
but it's the simplest.
Regards,
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v2] mem/hugetlb: shift an empty region to use in hugemmap02.c
2015-10-13 10:54 ` Jan Stancek
@ 2015-10-13 11:11 ` Jan Stancek
0 siblings, 0 replies; 8+ messages in thread
From: Jan Stancek @ 2015-10-13 11:11 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Jan Stancek" <jstancek@redhat.com>
> To: "Li Wang" <liwang@redhat.com>, "Cyril Hrubis" <chrubis@suse.cz>
> Cc: ltp@lists.linux.it
> Sent: Tuesday, 13 October, 2015 12:54:46 PM
> Subject: Re: [LTP] [PATCH v2] mem/hugetlb: shift an empty region to use in hugemmap02.c
>
> The approach above should work, but it also stores data to buf,
> that we don't care about. You could do fscanf + "while (fgetc(f) != '\n');"
> to forward to next line, if you want to avoid using buf.
OK, I take that back, Cyril's version is prettier:
>
> ret = fscanf(f, "%lx-%lx %*[^\n]\n", &start, &end);
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v2] mem/hugetlb: shift an empty region to use in hugemmap02.c
2015-10-13 10:53 ` Cyril Hrubis
@ 2015-10-13 13:39 ` Li Wang
0 siblings, 0 replies; 8+ messages in thread
From: Li Wang @ 2015-10-13 13:39 UTC (permalink / raw)
To: ltp
Hi,
On Tue, Oct 13, 2015 at 6:53 PM, Cyril Hrubis <chrubis@suse.cz> wrote:
> Hi!
> > okay, seems you are right. I tried and achieved another one, following
> > code test pass on my s390x system.
> > -----------------------------------------
> > int range_is_mapped(unsigned long low, unsigned long high)
> > {
> > FILE *f = SAFE_FOPEN(NULL, "/proc/self/maps", "r");
>
> If we start to use SAFE_MACROS() here we have to pass the cleanup
> callback to the range_is_mapped() as a first parameter so that we can
> pass correct cleanup callback to the SAFE_MACROS().
>
ok, now I prefer to use fopen()/fclose() to keep simple.
>
> > char buf[512];
> >
> > while (!feof(f)) {
> > unsigned long start, end;
> > int ret;
> >
> > ret = fscanf(f, "%lx-%lx %[^\n]%*c]", &start, &end, buf);
>
> You can do even better with:
>
> ret = fscanf(f, "%lx-%lx %*[^\n]\n", &start, &end);
>
great!
>
> Which says read two long ints and then ignore everything until the end
> of line.
>
> You could have ommited the last '\n' in the format string, but then you
> would need to exit the loop when the fscanf() returns -1 since the
> feof() would not exit the loop since there would still be the last
> newline in the buffer after we read the last line...
>
ok, agreed. I will sent V3.
thanks for reviewing the patch.
--
Regards,
Li Wang
Email: liwang@redhat.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20151013/d3351495/attachment.html>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-10-13 13:39 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-12 10:53 [LTP] [PATCH v2] mem/hugetlb: shift an empty region to use in hugemmap02.c Li Wang
2015-10-12 18:29 ` Cyril Hrubis
2015-10-13 5:39 ` Li Wang
2015-10-13 10:15 ` Li Wang
2015-10-13 10:53 ` Cyril Hrubis
2015-10-13 13:39 ` Li Wang
2015-10-13 10:54 ` Jan Stancek
2015-10-13 11:11 ` Jan Stancek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox