[PATCH]fix VM_CAN_NONLINEAR check in sys_remap_file_pages

2007-10-08 Thread Yan Zheng
Hi all

The test for VM_CAN_NONLINEAR always fails

Signed-off-by: Yan Zheng[EMAIL PROTECTED]

diff -ur linux-2.6.23-rc9/mm/fremap.c linux/mm/fremap.c
--- linux-2.6.23-rc9/mm/fremap.c2007-10-07 15:03:33.0 +0800
+++ linux/mm/fremap.c   2007-10-08 19:33:44.0 +0800
@@ -160,7 +160,7 @@
if (vma-vm_private_data  !(vma-vm_flags  VM_NONLINEAR))
goto out;

-   if (!vma-vm_flags  VM_CAN_NONLINEAR)
+   if (!(vma-vm_flags  VM_CAN_NONLINEAR))
goto out;

if (end = start || start  vma-vm_start || end  vma-vm_end)
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH]fix VM_CAN_NONLINEAR check in sys_remap_file_pages

2007-10-08 Thread Andrew Morton
On Mon, 8 Oct 2007 19:45:08 +0800 Yan Zheng [EMAIL PROTECTED] wrote:

 Hi all
 
 The test for VM_CAN_NONLINEAR always fails
 
 Signed-off-by: Yan Zheng[EMAIL PROTECTED]
 
 diff -ur linux-2.6.23-rc9/mm/fremap.c linux/mm/fremap.c
 --- linux-2.6.23-rc9/mm/fremap.c  2007-10-07 15:03:33.0 +0800
 +++ linux/mm/fremap.c 2007-10-08 19:33:44.0 +0800
 @@ -160,7 +160,7 @@
   if (vma-vm_private_data  !(vma-vm_flags  VM_NONLINEAR))
   goto out;
 
 - if (!vma-vm_flags  VM_CAN_NONLINEAR)
 + if (!(vma-vm_flags  VM_CAN_NONLINEAR))
   goto out;
 
   if (end = start || start  vma-vm_start || end  vma-vm_end)

Lovely.  From this we can deduce that nobody has run remap_file_pages() since
2.6.23-rc1 and that nobody (including the developer who made that change) ran it
while that change was in -mm.

I'm surprise that LTP doesn't have any remap_file_pages() tests.

Have you runtime tested this change?

Thanks.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH]fix VM_CAN_NONLINEAR check in sys_remap_file_pages

2007-10-08 Thread Randy Dunlap
On Mon, 8 Oct 2007 10:04:56 -0700 Andrew Morton wrote:

 On Mon, 8 Oct 2007 19:45:08 +0800 Yan Zheng [EMAIL PROTECTED] wrote:
 
  Hi all
  
  The test for VM_CAN_NONLINEAR always fails
  
  Signed-off-by: Yan Zheng[EMAIL PROTECTED]
  
  diff -ur linux-2.6.23-rc9/mm/fremap.c linux/mm/fremap.c
  --- linux-2.6.23-rc9/mm/fremap.c2007-10-07 15:03:33.0 +0800
  +++ linux/mm/fremap.c   2007-10-08 19:33:44.0 +0800
  @@ -160,7 +160,7 @@
  if (vma-vm_private_data  !(vma-vm_flags  VM_NONLINEAR))
  goto out;
  
  -   if (!vma-vm_flags  VM_CAN_NONLINEAR)
  +   if (!(vma-vm_flags  VM_CAN_NONLINEAR))
  goto out;
  
  if (end = start || start  vma-vm_start || end  vma-vm_end)
 
 Lovely.  From this we can deduce that nobody has run remap_file_pages() since
 2.6.23-rc1 and that nobody (including the developer who made that change) ran 
 it
 while that change was in -mm.

I've run rmap-test with -M (use remap_file_pages) and
remap-test from ext3-tools, but not remap_file_pages for some reason.

I'll now add remap_file_pages soon.
Maybe those other 2 tests aren't strong enough (?).
Or maybe they don't return a non-0 exit status even when they fail...
(I'll check.)


 I'm surprise that LTP doesn't have any remap_file_pages() tests.

quick grep didn't find any for me.

 Have you runtime tested this change?
 
 Thanks.


---
~Randy
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH]fix VM_CAN_NONLINEAR check in sys_remap_file_pages

2007-10-08 Thread Andrew Morton
On Mon, 8 Oct 2007 10:28:43 -0700
Randy Dunlap [EMAIL PROTECTED] wrote:

 On Mon, 8 Oct 2007 10:04:56 -0700 Andrew Morton wrote:
 
  On Mon, 8 Oct 2007 19:45:08 +0800 Yan Zheng [EMAIL PROTECTED] wrote:
  
   Hi all
   
   The test for VM_CAN_NONLINEAR always fails
   
   Signed-off-by: Yan Zheng[EMAIL PROTECTED]
   
   diff -ur linux-2.6.23-rc9/mm/fremap.c linux/mm/fremap.c
   --- linux-2.6.23-rc9/mm/fremap.c  2007-10-07 15:03:33.0 +0800
   +++ linux/mm/fremap.c 2007-10-08 19:33:44.0 +0800
   @@ -160,7 +160,7 @@
 if (vma-vm_private_data  !(vma-vm_flags  VM_NONLINEAR))
 goto out;
   
   - if (!vma-vm_flags  VM_CAN_NONLINEAR)
   + if (!(vma-vm_flags  VM_CAN_NONLINEAR))
 goto out;
   
 if (end = start || start  vma-vm_start || end  vma-vm_end)
  
  Lovely.  From this we can deduce that nobody has run remap_file_pages() 
  since
  2.6.23-rc1 and that nobody (including the developer who made that change) 
  ran it
  while that change was in -mm.
 
 I've run rmap-test with -M (use remap_file_pages) and
 remap-test from ext3-tools, but not remap_file_pages for some reason.
 
 I'll now add remap_file_pages soon.
 Maybe those other 2 tests aren't strong enough (?).
 Or maybe they don't return a non-0 exit status even when they fail...
 (I'll check.)

Perhaps Yan Zheng can tell us what test was used to demonstrate this?

 
  I'm surprise that LTP doesn't have any remap_file_pages() tests.
 
 quick grep didn't find any for me.

Me either.  There are a few lying around the place which could be
integrated.

It would be good if LTP were to have some remap_file_pages() tests
(please).  As we see here, it is something which we can easily break, and
leave broken for some time.

-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH]fix VM_CAN_NONLINEAR check in sys_remap_file_pages

2007-10-08 Thread Hugh Dickins
On Mon, 8 Oct 2007, Yan Zheng wrote:
 
 The test for VM_CAN_NONLINEAR always fails

Good catch indeed.  Though I was puzzled how we do nonlinear at all,
until I realized it's The test for not VM_CAN_NONLINEAR always fails.

It's not as serious as it appears, since code further down has been
added more recently to simulate nonlinear on non-RAM-backed filesystems,
instead of going the real nonlinear way; so most filesystems are now not
required to do what VM_CAN_NONLINEAR was put in to ensure they could do.

I'm confused as to where that leaves us: is this actually a fix that
needs to go into 2.6.23?  or will it suddenly disable a system call
which has been silently working fine on various filesystems which did
not add VM_CAN_NONLINEAR?  could we just rip out VM_CAN_NONLINEAR?
I hope Nick or Miklos is clearer on what the risks are.

(Apologies for all the nots and nons here, I'm embarrassed
after just criticizing Ingo's SCHED_NO_NO_OMIT_FRAME_POINTER!)

Hugh

 
 Signed-off-by: Yan Zheng[EMAIL PROTECTED]
 
 diff -ur linux-2.6.23-rc9/mm/fremap.c linux/mm/fremap.c
 --- linux-2.6.23-rc9/mm/fremap.c  2007-10-07 15:03:33.0 +0800
 +++ linux/mm/fremap.c 2007-10-08 19:33:44.0 +0800
 @@ -160,7 +160,7 @@
   if (vma-vm_private_data  !(vma-vm_flags  VM_NONLINEAR))
   goto out;
 
 - if (!vma-vm_flags  VM_CAN_NONLINEAR)
 + if (!(vma-vm_flags  VM_CAN_NONLINEAR))
   goto out;
 
   if (end = start || start  vma-vm_start || end  vma-vm_end)
 -
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
 
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH]fix VM_CAN_NONLINEAR check in sys_remap_file_pages

2007-10-08 Thread Yan Zheng
2007/10/8, Hugh Dickins [EMAIL PROTECTED]:
 On Mon, 8 Oct 2007, Yan Zheng wrote:
 
  The test for VM_CAN_NONLINEAR always fails
 Good catch indeed.  Though I was puzzled how we do nonlinear at all,
 until I realized it's The test for not VM_CAN_NONLINEAR always fails.
 It's not as serious as it appears, since code further down has been
 added more recently to simulate nonlinear on non-RAM-backed filesystems,
 instead of going the real nonlinear way; so most filesystems are now not
 required to do what VM_CAN_NONLINEAR was put in to ensure they could do.
 I'm confused as to where that leaves us: is this actually a fix that
 needs to go into 2.6.23?  or will it suddenly disable a system call
 which has been silently working fine on various filesystems which did
 not add VM_CAN_NONLINEAR?  could we just rip out VM_CAN_NONLINEAR?
 I hope Nick or Miklos is clearer on what the risks are.
 (Apologies for all the nots and nons here, I'm embarrassed
 after just criticizing Ingo's SCHED_NO_NO_OMIT_FRAME_POINTER!)
 Hugh

Yes, I mean The test for not VM_CAN_NONLINEAR always fails.  please
forgive my poor English.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH]fix VM_CAN_NONLINEAR check in sys_remap_file_pages

2007-10-08 Thread Nick Piggin
On Monday 08 October 2007 23:37, Hugh Dickins wrote:
 On Mon, 8 Oct 2007, Yan Zheng wrote:
  The test for VM_CAN_NONLINEAR always fails

 Good catch indeed.  Though I was puzzled how we do nonlinear at all,
 until I realized it's The test for not VM_CAN_NONLINEAR always fails.

 It's not as serious as it appears, since code further down has been
 added more recently to simulate nonlinear on non-RAM-backed filesystems,
 instead of going the real nonlinear way; so most filesystems are now not
 required to do what VM_CAN_NONLINEAR was put in to ensure they could do.

Well, I think all filesystems can do VM_CAN_NONLINEAR anyway. Device
drivers and weird things tend to have trouble...


 I'm confused as to where that leaves us: is this actually a fix that
 needs to go into 2.6.23?  or will it suddenly disable a system call
 which has been silently working fine on various filesystems which did
 not add VM_CAN_NONLINEAR?  could we just rip out VM_CAN_NONLINEAR?

We probably should keep VM_CAN_NONLINEAR for the moment, I think.
But now that we have the fallback path, we _could_ use that instead of
failing. I doubt anybody will be using nonlinear mappings on anything but
regular files for the time being, but as a trivial fix, I think this probably
should go into 2.6.23.

Thanks for spotting this problem
Acked-by: Nick Piggin [EMAIL PROTECTED]

 I hope Nick or Miklos is clearer on what the risks are.

 (Apologies for all the nots and nons here, I'm embarrassed
 after just criticizing Ingo's SCHED_NO_NO_OMIT_FRAME_POINTER!)

 Hugh

  Signed-off-by: Yan Zheng[EMAIL PROTECTED]
  
  diff -ur linux-2.6.23-rc9/mm/fremap.c linux/mm/fremap.c
  --- linux-2.6.23-rc9/mm/fremap.c2007-10-07 15:03:33.0 +0800
  +++ linux/mm/fremap.c   2007-10-08 19:33:44.0 +0800
  @@ -160,7 +160,7 @@
  if (vma-vm_private_data  !(vma-vm_flags  VM_NONLINEAR))
  goto out;
 
  -   if (!vma-vm_flags  VM_CAN_NONLINEAR)
  +   if (!(vma-vm_flags  VM_CAN_NONLINEAR))
  goto out;
 
  if (end = start || start  vma-vm_start || end  vma-vm_end)
  -
  To unsubscribe from this list: send the line unsubscribe linux-kernel
  in the body of a message to [EMAIL PROTECTED]
  More majordomo info at  http://vger.kernel.org/majordomo-info.html
  Please read the FAQ at  http://www.tux.org/lkml/
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH]fix VM_CAN_NONLINEAR check in sys_remap_file_pages

2007-10-08 Thread Nick Piggin
On Tuesday 09 October 2007 03:04, Andrew Morton wrote:
 On Mon, 8 Oct 2007 19:45:08 +0800 Yan Zheng [EMAIL PROTECTED] wrote:
  Hi all
 
  The test for VM_CAN_NONLINEAR always fails
 
  Signed-off-by: Yan Zheng[EMAIL PROTECTED]
  
  diff -ur linux-2.6.23-rc9/mm/fremap.c linux/mm/fremap.c
  --- linux-2.6.23-rc9/mm/fremap.c2007-10-07 15:03:33.0 +0800
  +++ linux/mm/fremap.c   2007-10-08 19:33:44.0 +0800
  @@ -160,7 +160,7 @@
  if (vma-vm_private_data  !(vma-vm_flags  VM_NONLINEAR))
  goto out;
 
  -   if (!vma-vm_flags  VM_CAN_NONLINEAR)
  +   if (!(vma-vm_flags  VM_CAN_NONLINEAR))
  goto out;
 
  if (end = start || start  vma-vm_start || end  vma-vm_end)

 Lovely.  From this we can deduce that nobody has run remap_file_pages()
 since 2.6.23-rc1 and that nobody (including the developer who made that
 change) ran it while that change was in -mm.

But you'd be wrong. remap_file_pages was tested both with my own tester
and Ingo's test program.

vm_flags != 0, !vm_flags = 0, 0  x = 0, so the test always falls
through. Of course, what I _should_ have done is also test a driver which
does not have VM_CAN_NONLINEAR... but even I wouldn't rewrite half
the nonlinear mapping code without once testing it ;)

FWIW, Oracle (maybe the sole real user of this) has been testing it, which
I'm very happy about (rather than testing after 2.6.23 is released).
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH]fix VM_CAN_NONLINEAR check in sys_remap_file_pages

2007-10-08 Thread Nick Piggin
On Tuesday 09 October 2007 03:51, Andrew Morton wrote:
 On Mon, 8 Oct 2007 10:28:43 -0700

  I'll now add remap_file_pages soon.
  Maybe those other 2 tests aren't strong enough (?).
  Or maybe they don't return a non-0 exit status even when they fail...
  (I'll check.)

 Perhaps Yan Zheng can tell us what test was used to demonstrate this?

Was probably found by review. Otherwise, you could probably reproduce
it by mmaping, say, drm device node, running remap_file_pages() on it
to create a nonlinear mapping, and then finding that you get the wrong
data.

   I'm surprise that LTP doesn't have any remap_file_pages() tests.
 
  quick grep didn't find any for me.

 Me either.  There are a few lying around the place which could be
 integrated.

 It would be good if LTP were to have some remap_file_pages() tests
 (please).  As we see here, it is something which we can easily break, and
 leave broken for some time.

Here is Ingo's old test, since cleaned up and fixed a bit by me
I'm sure he would distribute it GPL, but I've cc'ed him because I didn't
find an explicit statement about that.

/*
 * Copyright (C) Ingo Molnar, 2002
 */
#define _GNU_SOURCE
#include stdio.h
#include unistd.h
#include sys/mman.h
#include sys/stat.h
#include sys/types.h
#include fcntl.h
#include errno.h
#include stdlib.h
#include sys/times.h
#include sys/wait.h
#include sys/ioctl.h
#include sys/syscall.h
#include linux/unistd.h

#define PAGE_SIZE 4096
#define PAGE_WORDS (PAGE_SIZE/sizeof(int))

#define CACHE_PAGES 1024
#define CACHE_SIZE (CACHE_PAGES*PAGE_SIZE)

#define WINDOW_PAGES 16
#define WINDOW_SIZE (WINDOW_PAGES*PAGE_SIZE)

#define WINDOW_START 0x4800

static char cache_contents [CACHE_SIZE];

static void test_nonlinear(int fd)
{
	char *data = NULL;
	int i, j, repeat = 2;

	for (i = 0; i  CACHE_PAGES; i++) {
		int *page = (int *) (cache_contents + i*PAGE_SIZE);

		for (j = 0; j  PAGE_WORDS; j++)
			page[j] = i;
	}

	if (write(fd, cache_contents, CACHE_SIZE) != CACHE_SIZE)
		perror(write), exit(1);

	data = mmap((void *)WINDOW_START,
			WINDOW_SIZE,
			PROT_READ|PROT_WRITE, 
			MAP_FIXED | MAP_SHARED 
			, fd, 0);

	if (data == MAP_FAILED)
		perror(mmap), exit(1);

again:
	for (i = 0; i  WINDOW_PAGES; i += 2) {
		char *page = data + i*PAGE_SIZE;

		if (remap_file_pages(page, PAGE_SIZE * 2, 0,
(WINDOW_PAGES-i-2), 0) == -1)
			perror(remap_file_pages), exit(1);
	}

	for (i = 0; i  WINDOW_PAGES; i++) {
		/*
		 * Double-check the correctness of the mapping:
		 */
		if (i  1) {
			if (data[i*PAGE_SIZE] != WINDOW_PAGES-i) {
printf(hm, mapped incorrect data!\n);
exit(1);
			}
		} else {
			if (data[i*PAGE_SIZE] != WINDOW_PAGES-i-2) {
printf(hm, mapped incorrect data!\n);
exit(1);
			}
		}
	}

	if (--repeat)
		goto again;
}

int main(int argc, char **argv)
{
	int fd;

	fd = open(/dev/shm/cache, O_RDWR|O_CREAT|O_TRUNC,S_IRWXU);
	if (fd  0)
		perror(open), exit(1);
	test_nonlinear(fd);
	if (close(fd) == -1)
		perror(close), exit(1);
	printf(nonlinear shm file OK\n);

	fd = open(/tmp/cache, O_RDWR|O_CREAT|O_TRUNC,S_IRWXU);
	if (fd  0)
		perror(open), exit(1);
	test_nonlinear(fd);
	if (close(fd) == -1)
		perror(close), exit(1);
	printf(nonlinear /tmp/ file OK\n);

	exit(0);
}



Re: [PATCH]fix VM_CAN_NONLINEAR check in sys_remap_file_pages

2007-10-08 Thread Yan Zheng
2007/10/9, Andrew Morton [EMAIL PROTECTED]:
 Perhaps Yan Zheng can tell us what test was used to demonstrate this?

I found it by review, only do test to check remap_file_pages works
when VM_CAN_NONLINEAR flags is set.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html