On Thu, 20 Dec 2007, Peter Zijlstra wrote:
>
> Lennart asked for madvise(WILLNEED) to work on anonymous pages, he plans
> to use this to pre-fault pages. He currently uses: mlock/munlock for
> this purpose.
I certainly agree with this in principle: it just seems an unnecessary
and surprising restriction to refuse on anonymous vmas; I guess the only
reason for not adding this was not having anyone asking for it until now.
Though, does Lennart realize he could use MAP_POPULATE in the mmap?
>
> [ compile tested only ]
I haven't tried it either, but generally it looks plausible.
>
> Signed-off-by: Peter Zijlstra <[EMAIL PROTECTED]>
> ---
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 93ee375..eff60ce 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -100,6 +100,24 @@ out:
> return error;
> }
>
> +static long madvice_willneed_anon(struct vm_area_struct *vma,
> + struct vm_area_struct **prev,
> + unsigned long start, unsigned long end)
mavise.c uses "madvise_" rather than " madvice_" throughout,
so please go with the flow.
> +{
> + int ret, len;
> +
> + *prev = vma;
> + if (end > vma->vm_end)
> + end = vma->vm_end;
Please check, but I think the upper level ensures end is within range.
> +
> + len = end - start;
> + ret = get_user_pages(current, current->mm, start, len,
> + 0, 0, NULL, NULL);
> + if (ret < 0)
> + return ret;
> + return ret == len ? 0 : -1;
It's not good to return -1 as an alternative to a real errno:
it'll look like -EPERM. If you copied that from somewhere, better
send a patch to fix the somewhere! Ah, yes, make_pages_present: it
happens that nobody is interested in its return value, so we could
make it a void; but that'd just be a cleanup. What to do here if
non-negative ret less than len? Oh, just return 0, that's good
enough in this case (the file case always returns 0).
Hmm, might it be better to use make_pages_present itself,
fixing its retval, rather than using get_user_pages directly?
(I'd hope the caching makes its repeat of find_vma not an overhead.)
Interesting divergence: make_pages_present faults in writable pages
in a writable vma, whereas the file case's force_page_cache_readahead
doesn't even insert the pages into the mm.
> +}
> +
> /*
> * Schedule all required I/O operations. Do not wait for completion.
> */
> @@ -110,7 +128,7 @@ static long madvise_willneed(struct vm_area_struct * vma,
> struct file *file = vma->vm_file;
>
> if (!file)
> - return -EBADF;
> + return madvice_willneed_anon(vma, prev, start, end);
>
> if (file->f_mapping->a_ops->get_xip_page) {
> /* no bad return value, but ignore advice */
And there's a correctly invisible hunk to the patch too: this
extension of MADV_WILLNEED also does not require down_write of
mmap_sem, so madvise_need_mmap_write can remain unchanged.
Hugh
--
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/
