Re: [Patch] shmem_unuse race fix

From: Linus Torvalds (torvalds@transmeta.com)
Date: Wed Dec 27 2000 - 23:35:56 EST


On 27 Dec 2000, Christoph Rohland wrote:

> Marcelo Tosatti <marcelo@conectiva.com.br> writes:
>
> > I think that incrementing the swap entry count will not allow swap from
> > removing the swap entry (as the comment says)
>
> I think the culprit is somewhere else. The error occurs in nopage of a
> process, not in swapoff.

I think swapoff() should be fixed..

I moved the

        if (PageSwapCache(page))
                delete_from_swap_cache(page);

thing to _before_ the "unuse_process()" and "shmem_unuse()" code, because
I wanted to avoid a race on the PageDirty bit that way. However, that
opens up another race, the one you see with "nopage".

Woul dyou mind testing this alternate fix instead:

 - add the lines

        repeat:
                ClearPageDirty(page);

   to just before the "read_lock(&tasklist_lock);" in try_to_unuse(). We
   can obviously mark the page clean, because we _are_ going to get rid of
   it, and in the meantime we have a hold on it by virtue of having raised
   the page count in "read_swap_cache()".

 - move the "delete_from_swap_cache()" call back to _after_ the
   unuse() calls.

 - but just before deleting the entry, we add a new test:

        if (PageDirty(page))
                goto repeat;

this all should mean that if something moves the dirty bit from a page
table to the backing store, we will notice, and just re-do the VM scan,
which will mark the page table entry dirty again. And because we delete it
from the swap cache late, we aren't severing the link with
"nopage" handling.

Christoph, how does this sound to you?

                Linus

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



This archive was generated by hypermail 2b29 : Sun Dec 31 2000 - 21:00:10 EST