Re: [NEED TESTERS] remove swapin_readahead Re: shmem_getpage_locked() / swapin_readahead() race in 2.4.4-pre3

2001-04-18 Thread Christoph Rohland

Hi Stephen,

On Tue, 17 Apr 2001, Stephen C. Tweedie wrote:
> I don't see the problem.  shmem_getpage_locked appears to back off
> correctly if it encounters a swap-cached page already existing if
> swapin_readahead has installed the page first, at least with the
> code in 2.4.3-ac5.

But the swap count can be increased by anybody without having the page
lock. So the check triggers and is bogus. See my old patch.

> There *does* appear to be a race, but it's swapin_readahead racing
> with shmem_writepage.  That code does not search for an existing
> entry in the swap cache when it decides to move a shmem page to
> swap, so we can install the page twice and end up doing a lookup on
> the wrong physical page if there is swap readahead going on.

I cannot follow you here. How can we have a swap cache entry if there
is no swap entry. . . . Oh stop you mean swapin_readahead does swap in
some totally bogus page into the swap cache after we did
__get_swap_page? I never thought about that!

> To fix that, shmem_writepage needs to do a swap cache lookup and
> lock before installing the new page --- it should probably just copy
> the new page into the old one if it finds one already there.

OK I will look into that.

Greetings
Christoph


-
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/



Re: [NEED TESTERS] remove swapin_readahead Re: shmem_getpage_locked() / swapin_readahead() race in 2.4.4-pre3

2001-04-18 Thread Christoph Rohland

Hi Stephen,

On Tue, 17 Apr 2001, Stephen C. Tweedie wrote:
 I don't see the problem.  shmem_getpage_locked appears to back off
 correctly if it encounters a swap-cached page already existing if
 swapin_readahead has installed the page first, at least with the
 code in 2.4.3-ac5.

But the swap count can be increased by anybody without having the page
lock. So the check triggers and is bogus. See my old patch.

 There *does* appear to be a race, but it's swapin_readahead racing
 with shmem_writepage.  That code does not search for an existing
 entry in the swap cache when it decides to move a shmem page to
 swap, so we can install the page twice and end up doing a lookup on
 the wrong physical page if there is swap readahead going on.

I cannot follow you here. How can we have a swap cache entry if there
is no swap entry. . . . Oh stop you mean swapin_readahead does swap in
some totally bogus page into the swap cache after we did
__get_swap_page? I never thought about that!

 To fix that, shmem_writepage needs to do a swap cache lookup and
 lock before installing the new page --- it should probably just copy
 the new page into the old one if it finds one already there.

OK I will look into that.

Greetings
Christoph


-
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/



Re: [NEED TESTERS] remove swapin_readahead Re: shmem_getpage_locked() / swapin_readahead() race in 2.4.4-pre3

2001-04-17 Thread Stephen C. Tweedie

Hi,

On Sat, Apr 14, 2001 at 08:31:07PM -0300, Marcelo Tosatti wrote:
> On Sat, 14 Apr 2001, Rik van Riel wrote:
> > On Sat, 14 Apr 2001, Marcelo Tosatti wrote:
> > 
> > > There is a nasty race between shmem_getpage_locked() and
> > > swapin_readahead() with the new shmem code (introduced in
> > > 2.4.3-ac3 and merged in the main tree in 2.4.4-pre3):

> Test (multiple shm-stress) runs fine without swapin_readahead(), as
> expected.

> Stephen/Linus? 

I don't see the problem.  shmem_getpage_locked appears to back off
correctly if it encounters a swap-cached page already existing if
swapin_readahead has installed the page first, at least with the code
in 2.4.3-ac5.

There *does* appear to be a race, but it's swapin_readahead racing
with shmem_writepage.  That code does not search for an existing entry
in the swap cache when it decides to move a shmem page to swap, so we
can install the page twice and end up doing a lookup on the wrong
physical page if there is swap readahead going on.

To fix that, shmem_writepage needs to do a swap cache lookup and lock
before installing the new page --- it should probably just copy the
new page into the old one if it finds one already there.

--Stephen
-
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/