Re: Memory leak in 2.0.33 ??

Benjamin C.R. LaHaise (blah@kvack.org)
Tue, 23 Dec 1997 13:01:11 -0500 (EST)


On Tue, 23 Dec 1997, Dr. Werner Fink wrote:

> > Hi all...
> > It looks like the swap cache bug. I have posted a fix for it many times
> > to this list and it was even fixed in pre-2.0.31-2, but things look bad.
> > Linus probably decided to keep the bug forever in the kernel. I suppose,
> > it is still present in the 2.1.xx kernels... If you have ram >= swap,
> > swap > 0 - be ready for crash!
> > You can find the fix (for older kernel, but it should be applicable to the
> > newer too) on Linux Mama (http://www.huwig.de/linux/mama).
> > Krzysztof Strasburger
> >
>
> Hmmm ... the win of caching pages recently swapped out should not be breaked
> by handle this cached pages as free pages.

I wonder if he might be seeing another bug I came across in all recent
kernels - kswapd will not swap out any shared privately mapped pages.

Note this section of code in mm/vmscan.c:try_to_swap_out (from 2.1.66 if
anyone cares)

if (pte_dirty(pte)) {
1---> if (vma->vm_ops && vma->vm_ops->swapout) {
pid_t pid = tsk->pid;
vma->vm_mm->rss--;
if (vma->vm_ops->swapout(vma, address - vma->vm_start + vma->vm_offset, page_table))
kill_proc(pid, SIGBUS, 1);
} else {
2--------> if (atomic_read(&page_map->count) != 1)
2--------> return 0;
if (!(entry = get_swap_page()))
return 0;
vma->vm_mm->rss--;

Now note this section of code from mm/memory.c:copy_one_pte

if (cow)
pte = pte_wrprotect(pte);
3----> if (delete_from_swap_cache(&mem_map[page_nr]))
3----> pte = pte_mkdirty(pte);
set_pte(new_pte, pte_mkold(pte));
set_pte(old_pte, pte);
atomic_inc(&mem_map[page_nr].count);

(1) will only be true for privately mapped pages. Let's say a big process
like inn does a fork(). Now you have a nice chunk of memory that will
never be swapped out until the region is unmapped. This behaviour is
normally masked by the fork() then exec() nature of most applications.

The fix is simply to remove the code at (2) - my own machines have been
runnning with such a patch for months, and no ill effects. It looks to be
a hold over from some plans to only write a page out to swap only once. A
slightly better fix would include a change at (3) to point the pte at the
swap entry (if still cached) instead of throwing it away. Hmmm, thinking
about it more, why even bother discarding the swap cache entry? Just
write protect the pte if it's in the swap cache.

Linus: could you comment on this?

-ben