Linus Torvalds wrote:
> Looks reasonable - although your "max_size" checks are wrong at mmap time.
> You should check for something like
thanks
> > Since this code works in my local tests, my two concerns at this point
> > are correct vma->vm_pgoff treatment, and correct vm_flags/vm_file usage.
Can I completely ignore vm_pgoff in nopage()? Currently I just do the
following, to get the page number relative to the start of the vma:
pgoff = (address - vma->vm_start) >> PAGE_SHIFT;
where 'address' is the value passed to nopage().
> You should drop the
>
> vma->vm_file = file;
>
> as the VM layer will have done that already.
>
> Also, the VM_LOCKED | VM_SHM thing is wrong, because it will cause
> mis-calulcation of the mm->vm_locked fields, and it's also unnecessary.
> Sure, it will cause the VM layer to not swap, but the no-op "swapping"
> shouldn't much hurt anyway, as it's easy enough to re-populate the page
> again.
>
> So either just drop it, or make sure that the locked page count doesn't
> get corrupted.
The first draft of code had only
vma->vm_ops = &via_mm_ops;
vma->vm_private_data = card;
and userspace apps segfaulted. I then added the two objectionable
lines, and things started working:
vma->vm_flags |= VM_LOCKED | VM_SHM; /* Don't swap */
vma->vm_file = file;
Have not yet checked any combinations in between...
Regards,
Jeff
--
Jeff Garzik | The difference between laziness and
Building 1024 | prioritization is the end result.
MandrakeSoft |
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
