Skip to contentSkip to conversation

mmap/mmap12.c: new case to test MAP_POPULATE of mmap

PATCH [email protected] 2 participants 3 messages Jun 25, 2013 – Jun 27, 2013 3 messages not in the archive

lore
Share & tools
lore

[email protected]

author
#1 lore raw
In reply to a message not in the archive

Hi!

Quoted 12 lines
(quoted diff) +12 -0
+static int page_check(void)
+{
+	int pid;
+	int ret;
+	int pm;
+	int num_pages;
+	long index;
+	off_t offset;
+	size_t page_sz;
+	char maps_dev[PATHLEN];
+	char pagemap_dev[PATHLEN];
+	char line[LINELEN];

Review

Quoted 10 lines
(quoted diff) +10 -0
+	FILE *maps;
+	unsigned long vm_start;
+	unsigned long vm_end;
+	unsigned long long pagemap;
+
+	page_sz = getpagesize();
+
+	pid = getpid();
+	sprintf(maps_dev, "/proc/%d/maps", pid);
+	sprintf(pagemap_dev, "/proc/%d/pagemap", pid);

Just use "/proc/self/maps" and "/proc/self/pagemap" instead.

Review

Quoted 11 lines
(quoted diff) +11 -0
+	maps = fopen(maps_dev, "r");
+	if (maps == NULL)
+		tst_brkm(TFAIL | TERRNO, NULL, "Open dev maps failed");
+
+	while (fgets(line, LINELEN, maps) != NULL)
+		if (strstr(line, TEMPFILE) != NULL)
+			break;
+
+	ret = sscanf(line, "%lX-%lX", &vm_start, &vm_end);
+	if (ret != 2)
+		tst_brkm(TFAIL | TERRNO, NULL, "Get address space failed");

This part of the code does not cope with unlikely condition that TEMPFILE is missing from /proc/self/maps we should fail the test in this case with proper error message.

Quoted 12 lines
(quoted diff) +12 -0
+	num_pages = (vm_end - vm_start) / page_sz;
+	index = (vm_start / page_sz) * sizeof(unsigned long long);
+
+	pm = open(pagemap_dev, O_RDONLY);
+	if (pm == -1)
+		tst_brkm(TFAIL | TERRNO, NULL, "Open dev pagemap failed");
+
+	offset = lseek(pm, index, SEEK_SET);
+	if (offset != index)
+		tst_brkm(TFAIL | TERRNO, NULL, "Reposition offset failed");
+
+	while (num_pages > 0) {

Review

Quoted 10 lines
(quoted diff) +10 -0
+		ret = read(pm, &pagemap, sizeof(unsigned long long));
+		if (ret < 0)
+			tst_brkm(TFAIL | TERRNO, NULL, "Read pagemap failed");
+		/*
+		 * Check if the page is present.
+		 */
+		if (!(pagemap & (1ULL<<63))) {
+			close(pm);
+			fclose(maps);
+			return 1;

I would perpahs be more verbose here and go over all of the pages and list these that are not mapped, but that is a minor nitpick.

Quoted 9 lines
(quoted diff) +9 -0
+		}
+		num_pages--;
+	}
+
+	close(pm);
+	fclose(maps);
+
+	return 0;
+}
Signature · 13 lines
Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
#5 lore raw
In reply to a message not in the archive
Quoted 55 lines · Jan Stancek

----- Original Message -----

Quoted 22 lines

From: "DAN LI" <[email protected]>
To: "LTP list" <[email protected]>
Cc: "Jan Stancek" <[email protected]>, [email protected], "Caspar Zhang" <[email protected]>
Sent: Tuesday, 25 June, 2013 11:13:27 AM
Subject: [LTP] [PATCH v2] mmap/mmap12.c: new case to test MAP_POPULATE of mmap

Create test case for MAP_POPULATE of mmap.

Verify the following statement
"For a file mapping,this causes read-ahead on the file.
Later accesses to the mapping will not be blocked by page faults."
by mmap-ing a file and check if all pages are present.

Signed-off-by DAN LI <[email protected]>
---
 runtest/syscalls                        |   1 +
 testcases/kernel/syscalls/mmap/mmap12.c | 194
 ++++++++++++++++++++++++++++++++
 2 files changed, 195 insertions(+)
 create mode 100644 testcases/kernel/syscalls/mmap/mmap12.c

<snip>

Quoted 28 lines
(quoted diff) +28 -0
+
+static int page_check(void)
+{
+	int pid;
+	int ret;
+	int pm;
+	int num_pages;
+	long index;
+	off_t offset;
+	size_t page_sz;
+	char maps_dev[PATHLEN];
+	char pagemap_dev[PATHLEN];
+	char line[LINELEN];
+	FILE *maps;
+	unsigned long vm_start;
+	unsigned long vm_end;
+	unsigned long long pagemap;
+
+	page_sz = getpagesize();
+
+	pid = getpid();
+	sprintf(maps_dev, "/proc/%d/maps", pid);
+	sprintf(pagemap_dev, "/proc/%d/pagemap", pid);
+
+	maps = fopen(maps_dev, "r");
+	if (maps == NULL)
+		tst_brkm(TFAIL | TERRNO, NULL, "Open dev maps failed");
+
Quoted 8 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+	while (fgets(line, LINELEN, maps) != NULL)
+		if (strstr(line, TEMPFILE) != NULL)
+			break;
+
+	ret = sscanf(line, "%lX-%lX", &vm_start, &vm_end);

Why parsing these 2 from maps? Size is defined by you, and you get vm_start from mmap().

Just a force of inertia following pagemap document.

Quoted 9 lines

Review

Quoted 6 lines
(quoted diff) +5 -0
+	if (ret != 2)
+		tst_brkm(TFAIL | TERRNO, NULL, "Get address space failed");
+
+	num_pages = (vm_end - vm_start) / page_sz;
+	index = (vm_start / page_sz) * sizeof(unsigned long long);

C99 says long long is _at least_ 64 bits wide. If it's larger than 64, formula above would break. How about using type, that's exactly 64 bit (uint64_t)?

Get it.

Quoted 32 lines
Quoted 32 lines
(quoted diff) +31 -0
+
+	pm = open(pagemap_dev, O_RDONLY);
+	if (pm == -1)
+		tst_brkm(TFAIL | TERRNO, NULL, "Open dev pagemap failed");
+
+	offset = lseek(pm, index, SEEK_SET);
+	if (offset != index)
+		tst_brkm(TFAIL | TERRNO, NULL, "Reposition offset failed");
+
+	while (num_pages > 0) {
+		ret = read(pm, &pagemap, sizeof(unsigned long long));
+		if (ret < 0)
+			tst_brkm(TFAIL | TERRNO, NULL, "Read pagemap failed");
+		/*
+		 * Check if the page is present.
+		 */
+		if (!(pagemap & (1ULL<<63))) {
+			close(pm);
+			fclose(maps);
+			return 1;
+		}
+		num_pages--;
+	}
+
+	close(pm);
+	fclose(maps);
+
+	return 0;
+}
+
+static void setup(void)
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+{
+	tst_sig(FORK, DEF_HANDLER, cleanup);
+
+	if ((tst_kvercmp(2, 6, 25)) < 0)
+		tst_brkm(TCONF, NULL,
+			"This test can only run on kernels that are 2.6.23 and "

Message says 2.6.23, but check is for 2.6.25.

Wow...

All of the above will be fixed in coming V3.

Thank you for reviewing.

Regards,
DAN LI

Quoted 17 lines
Quoted 17 lines
(quoted diff) +16 -0
+			"higher");
+
+	TEST_PAUSE;
+
+	tst_tmpdir();
+
+	fildes = open(TEMPFILE, O_RDWR | O_CREAT, 0766);
+	if (fildes < 0)
+		tst_brkm(TFAIL, cleanup, "opening %s failed", TEMPFILE);
+
+	if (ftruncate(fildes, MMAPSIZE) < 0)
+		tst_brkm(TFAIL | TERRNO, cleanup, "ftruncate file failed");
+
+}
+
+static void cleanup(void)
Quoted 9 lines
Quoted 8 lines
(quoted diff) +5 -1
+{
+	close(fildes);
+	TEST_CLEANUP;
+	tst_rmdir();
+}
--

1.8.1

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

author
#3 lore raw
In reply to a message not in the archive
Quoted 16 lines
Quoted 6 lines
Quoted 6 lines
(quoted diff) +6 -0
+	while (num_pages > 0) {
+		ret = read(pm, &pagemap, sizeof(unsigned long long));
+		if (ret < 0)
+			tst_brkm(TFAIL | TERRNO, NULL, "Read pagemap failed");
+		/*
+		 * Check if the page is present.
Quoted 9 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+		 */
+		if (!(pagemap & (1ULL<<63))) {
+			close(pm);
+			fclose(maps);
+			return 1;

I would perpahs be more verbose here and go over all of the pages and list these that are not mapped, but that is a minor nitpick.

Quoted 1 line

You mean listing every page which is not present?

Yes please, nothing fancy just short message with address of the page and its ordering number in the mapping printed using tst_resm with TINFO.

Signature · 13 lines
Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list