Re: [PATCH] Bug #3054 madvise(MADV_WILLNEED,...) fix for exceeding rlimit rss

[Chris Wright <chrisw@osdl.org>]

* Matt Keenan (matthew.keenan@ntlworld.com) wrote:
> --- linux-2.6.11.7/mm/madvise.c 2005-04-12 15:58:30.000000000 +0100
> +++ linux/mm/madvise.c  2005-06-19 17:20:56.000000000 +0100
> @@ -61,6 +61,7 @@ static long madvise_willneed(struct vm_a
>                             unsigned long start, unsigned long end)
> {
>        struct file *file = vma->vm_file;
> +       struct task_struct *tsk = current;

Looks like you've got some tab/whitespace damage going on here.

>        if (!file)
>                return -EBADF;
> @@ -70,6 +71,28 @@ static long madvise_willneed(struct vm_a
>                end = vma->vm_end;
>        end = ((end - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff;
> 
> +       /*
> +        * This code below checks to see if mapping the requested
> +        * readahead would make the task's rss exceed the task's
> +        * rlimit rss.
> +        *
> +        * This doesn't account for pages that may already be mapped
> +        * due to readahead, but since this is merely a hint to the
> +        * kernel no harm should be done, it won't unmap anything
> +        * already mapped if it fails. N.B. This won't affect the
> +        * kernel's internal automatic readahead which doesn't check
> +        * (or honour) rlimit rss.
> +        */
> +
> +       spin_lock(&tsk->mm->page_table_lock);
> +       if (((max_sane_readahead(end-start) << PAGE_SHIFT) +
> +           tsk->mm->_rss) > tsk->signal->rlim[RLIMIT_RSS].rlim_cur)

I doubt this one would overflow, but we recenly made changes in similar
tests to use page count rather than byte count.  I belive this should
use get_mm_counter().  Isn't _rss counting pages rather than bytes,
so I think the math is off here.  Something like:

	if ((max_sane_readahead(end - start) + get_mm_counter(tsk->mm, rss)) >
	    tsk->signal->rlim[RLIMIT_RSS].rlim_cur >> PAGE_SHIFT)

thanks,
-chris
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/