[patch] VM accounting change

[patch] VM accounting change

[Terence Ripperda]

[-- Attachment #1: Type: text/plain, Size: 940 bytes --]

I apologize for not having more first hand info on this problem.

I've been told that recent changes in vm accounting in mmap have
caused some problems with our driver during mmap.

the problem seems to stem from us oring vma->vm_flags w/ (VM_LOCKED 
| VM_IO). we do this for agp and pci pages that are mapped to push
buffers. VM_LOCKED since we don't want the pages to move and VM_IO so
they are not dumped during a core dump.

I've been told that the new vm accounting changes call
make_pages_present() for all pages in a vma range marked VM_LOCKED and
that this causes problems with our driver. the attached patch modifies
this to not call make_pages_present() if VM_IO is also set.

what I'm unclear on is if this is a valid fix. should
make_pages_present work for memory ranges that are already mapped and
locked down (via PG_reserved)? are we wrong in using (VM_IO |
VM_LOCKED) for our pages? is this fix correct?

Thanks,
Terence




[-- Attachment #2: 2.6.10-rc1-bk8-VM_IO.diff --]
[-- Type: text/plain, Size: 541 bytes --]

diff -ru linux-2.6.10-rc1-bk8/mm/mmap.c linux-2.6.10-rc1-bk8-2/mm/mmap.c
--- linux-2.6.10-rc1-bk8/mm/mmap.c	2004-11-06 15:04:28.000000000 +0100
+++ linux-2.6.10-rc1-bk8-2/mm/mmap.c	2004-11-06 15:39:47.000000000 +0100
@@ -1011,7 +1011,8 @@
 	__vm_stat_account(mm, vm_flags, file, len >> PAGE_SHIFT);
 	if (vm_flags & VM_LOCKED) {
 		mm->locked_vm += len >> PAGE_SHIFT;
-		make_pages_present(addr, addr + len);
+		if (!(vm_flags & VM_IO))
+			make_pages_present(addr, addr + len);
 	}
 	if (flags & MAP_POPULATE) {
 		up_write(&mm->mmap_sem);

Re: [patch] VM accounting change

[Andrew Morton]

Terence Ripperda <tripperda@nvidia.com> wrote:
>
> I've been told that recent changes in vm accounting in mmap have
>  caused some problems with our driver during mmap.
> 
>  the problem seems to stem from us oring vma->vm_flags w/ (VM_LOCKED 
>  | VM_IO). we do this for agp and pci pages that are mapped to push
>  buffers. VM_LOCKED since we don't want the pages to move and VM_IO so
>  they are not dumped during a core dump.

VM_LOCKED|VM_IO doesn't seem to be a sane combination.  VM_LOCKED means
"don't page it out" and VM_IO means "an IO region".  The kernel never even
attempts to page out IO regions because they don't have reverse mappings. 
Heck, they don't even have pageframes.

How about you drop the VM_LOCKED?

>  diff -ru linux-2.6.10-rc1-bk8/mm/mmap.c linux-2.6.10-rc1-bk8-2/mm/mmap.c
>  --- linux-2.6.10-rc1-bk8/mm/mmap.c	2004-11-06 15:04:28.000000000 +0100
>  +++ linux-2.6.10-rc1-bk8-2/mm/mmap.c	2004-11-06 15:39:47.000000000 +0100
>  @@ -1011,7 +1011,8 @@
>   	__vm_stat_account(mm, vm_flags, file, len >> PAGE_SHIFT);
>   	if (vm_flags & VM_LOCKED) {
>   		mm->locked_vm += len >> PAGE_SHIFT;
>  -		make_pages_present(addr, addr + len);
>  +		if (!(vm_flags & VM_IO))
>  +			make_pages_present(addr, addr + len);

Spose we could do that on the basis of "don't break existing drivers which
are doing peculiar things".

Re: [patch] VM accounting change

[Terence Ripperda]

On Thu, Nov 11, 2004 at 03:07:10PM -0800, akpm@osdl.org wrote:
> VM_LOCKED|VM_IO doesn't seem to be a sane combination.  VM_LOCKED means
> "don't page it out" and VM_IO means "an IO region".  The kernel never even
> attempts to page out IO regions because they don't have reverse mappings. 
> Heck, they don't even have pageframes.
> 
> How about you drop the VM_LOCKED?

sounds good, I can do that.

on a related note, there are a couple of flags that I'm not 100% clear
on the difference between, mainly:

VM_LOCKED
PG_locked
PG_reserved

everything I've seen in the past has suggested that drivers set the
PG_reserved flag for memory allocations intended to be locked down in
memory for extensive dma (the bttv driver had always been pointed to
as an example of that).

I'm not clear how that differs from PG_locked and VM_LOCKED. is
PG_reserved still the suggested way to properly lock memory down, or
is there a more generally accepted method?

Thanks,
Terence

Re: [patch] VM accounting change

[Andrew Morton]

Terence Ripperda <tripperda@nvidia.com> wrote:
>
> On Thu, Nov 11, 2004 at 03:07:10PM -0800, akpm@osdl.org wrote:
> > VM_LOCKED|VM_IO doesn't seem to be a sane combination.  VM_LOCKED means
> > "don't page it out" and VM_IO means "an IO region".  The kernel never even
> > attempts to page out IO regions because they don't have reverse mappings. 
> > Heck, they don't even have pageframes.
> > 
> > How about you drop the VM_LOCKED?
> 
> sounds good, I can do that.
> 
> on a related note, there are a couple of flags that I'm not 100% clear
> on the difference between, mainly:
> 
> VM_LOCKED
> PG_locked
> PG_reserved
> 
> everything I've seen in the past has suggested that drivers set the
> PG_reserved flag for memory allocations intended to be locked down in
> memory for extensive dma (the bttv driver had always been pointed to
> as an example of that).
> 
> I'm not clear how that differs from PG_locked and VM_LOCKED. is
> PG_reserved still the suggested way to properly lock memory down, or
> is there a more generally accepted method?

VM_LOCKED means that someone did mlock() and the VMA isn't eligible for
paging.

PG_locked is very different: it provides the caller with exclusive access
the page while its actual contents are being changed.  It's also used as a
synchronisation point for adding to and removing from pagecache.  It's
pretty much a pagecache concept rather than an MM concept.

PG_reserved does mean that the page is "special" and the VM should just
leave the thing alone - some device driver owns the page and knows how to
manage it.

VM_RESERVED is a bit of a mystery, really and we've had some trouble over
the semantics of this vs PG_reserved.  Presumably it's supposed to be like
PG_reserved, only for whole mmap regions.  It may not work properly because
it gets damn little testing.

We really should have gone through and rationalised, consolidated and
documented the PageReserved/VM_RESERVED code in the 2.5 cycle but it didn't
happen.  The most noxious part is all the testing of PG_reserved in the
core kernel page refcounting logic.