Re: [PATCH] kanoj-mm17-2.3.21 kswapd vma scanning protection

Manfred Spraul
1999-10-15T11:58:34Z

Kanoj Sarcar wrote:
> swapout() currently takes the vma as an input,
> but the vma might be getting deleted (the documentation which is
> part of the patch describes currently how things are protected),
> so it might be prudent to pass individual fields of the vma to the
> swapout() method, rather than a pointer to the structure.

passing the individual fields of the vma is impossible:
only swap_out knows which field of the vma are important, and which
locking is required (eg fget()).

AFAICS, there are only 2 acceptable solutions:
- lock_kernel() as in your patch.
- swap_out() is called with the semaphore held, and it sleeps with the
semaphore. [I prefer this solution: it's the first step towards swapping
without lock_kernel()].

Or: ->swapout() releases the semaphore, or split ->swapout() into 2
parts.

> + /*
> + * The lock_kernel interlocks with kswapd try_to_swap_out
> + * invoking a driver swapout() method, and being able to
> + * guarantee vma existance.
> + */
> lock_kernel();
> if (mpnt->vm_ops && mpnt->vm_ops->unmap)
> mpnt->vm_ops->unmap(mpnt, st, size);
> [...]
> flush_tlb_page(vma, address);
> + vmlist_access_unlock(vma->vm_mm);
> swap_duplicate(entry); /* One for the process, one for the swap cache */
>
> /* This will also lock the page */

I thought that the page stealer would call ->swapout() while owning the
vmlist_lock.
a) there should be no lock-up, because the swapper is never reentered
[PF_MEMALLOC].
b) noone except the swapper is allowed to sleep while owning
vmlist_lock.
c) getting rid of that lock_kernel() call is one of the main aims of the
vmlist_lock.

--
Manfred