Q: Swap Locking Reinstatement

Q: Swap Locking Reinstatement

[Eric W. Biederman]

Recently the swap lockmap has been readded.

Was that just as a low cost sanity check, to use especially while
there were bugs in some of the low level disk drivers?

Was there something that really needs the swap lockmap?

The reason I am asking is that this causes conflicts with my shmfs
kernel patches.  I directly read/write swap pages through a variation
of rw_swap_page, and during I/O they must stay in the page cache, but
_not_ on the swapper inode, and the way the swap lockmap is currently
implemented causes a problem.

Eric

Re: Q: Swap Locking Reinstatement

[Stephen C. Tweedie]

Hi,

On 12 May 1998 20:57:05 -0500, ebiederm+eric@npwt.net (Eric
W. Biederman) said:

> Recently the swap lockmap has been readded.

> Was that just as a low cost sanity check, to use especially while
> there were bugs in some of the low level disk drivers?

> Was there something that really needs the swap lockmap?

Yes, there was a bug.  The problem occurs when:

	page X is owned by process A
	process B tries to swap out page X from A's address space
	process A exits or execs
	process B's swap IO completes.

The IO completion is an interrupt (we perform async swaps where
possible).  Now, if we dereference the swap entry belonging to page X
at IO completion time, then the entry is protected against reuse while
the IO is in flight.  However, that requires making the entire swap map
interrupt safe.  It is much more efficient to keep the lock map separate
and to use atomic bitops on it to allow us to do the IO completion
unlock in an interrupt-safe manner.

A similar race occurs when

	process B tries to swap out page X from A's address space
	process A tries to swap it back in
	process B's swap IO completes.

Now process A may, or may not, get the right data from disk depending on
the (undefined) ordering of the IOs submitted by A and B.

> The reason I am asking is that this causes conflicts with my shmfs
> kernel patches.  I directly read/write swap pages through a variation
> of rw_swap_page, and during I/O they must stay in the page cache, but
> _not_ on the swapper inode, and the way the swap lockmap is currently
> implemented causes a problem.

Sorry, but right now it is definitely needed.

--Stephen

Re: Q: Swap Locking Reinstatement

[Eric W. Biederman]

>>>>> "ST" == Stephen C Tweedie <sct@dcs.ed.ac.uk> writes:

ST> Hi,
ST> On 12 May 1998 20:57:05 -0500, ebiederm+eric@npwt.net (Eric
ST> W. Biederman) said:

>> Recently the swap lockmap has been readded.

>> Was that just as a low cost sanity check, to use especially while
>> there were bugs in some of the low level disk drivers?

>> Was there something that really needs the swap lockmap?

ST> Yes, there was a bug.  The problem occurs when:

ST> 	page X is owned by process A
ST> 	process B tries to swap out page X from A's address space
ST> 	process A exits or execs
ST> 	process B's swap IO completes.

ST> The IO completion is an interrupt (we perform async swaps where
ST> possible).  Now, if we dereference the swap entry belonging to page X
ST> at IO completion time, then the entry is protected against reuse while
ST> the IO is in flight.  However, that requires making the entire swap map
ST> interrupt safe.  It is much more efficient to keep the lock map separate
ST> and to use atomic bitops on it to allow us to do the IO completion
ST> unlock in an interrupt-safe manner.

ST> A similar race occurs when

ST> 	process B tries to swap out page X from A's address space
ST> 	process A tries to swap it back in
ST> 	process B's swap IO completes.

ST> Now process A may, or may not, get the right data from disk depending on
ST> the (undefined) ordering of the IOs submitted by A and B.

Here is how I'm going to code it.

I'm going to modify swap_out to never remove a page from the page
cache until I/O is complete on it.  This should only affect
asynchrounous pages.

I'm going to modify shrink_mmap and friends so that when they remove
a swapper page from the page cache they will decrement the swap use
count, of the page. (Via a new generic inode function).

This should both remove the need for the swap lock map, and increase
performance on the second race condition you mentioned (because it
doesn't have to read the page back in).

Hopefully when we get reverse pte maps working we can remove the swap
use counts as well, and only worry if a swap page is in use.

Eric

Re: Q: Swap Locking Reinstatement

[Eric W. Biederman]

>>>>> "ST" == Stephen C Tweedie <sct@dcs.ed.ac.uk> writes:

Just to add another case to the list.

>> Recently the swap lockmap has been readded.

>> Was there something that really needs the swap lockmap?

ST> Yes, there was a bug.  The problem occurs when:

ST> 	page X is owned by process A
ST> 	process B tries to swap out page X from A's address space
ST> 	process A exits or execs
ST> 	process B's swap IO completes.

ST> The IO completion is an interrupt (we perform async swaps where
ST> possible).  Now, if we dereference the swap entry belonging to page X
ST> at IO completion time, then the entry is protected against reuse while
ST> the IO is in flight.  However, that requires making the entire swap map
ST> interrupt safe.  It is much more efficient to keep the lock map separate
ST> and to use atomic bitops on it to allow us to do the IO completion
ST> unlock in an interrupt-safe manner.

ST> A similar race occurs when

ST> 	process B tries to swap out page X from A's address space
ST> 	process A tries to swap it back in
ST> 	process B's swap IO completes.

ST> Now process A may, or may not, get the right data from disk depending on
ST> the (undefined) ordering of the IOs submitted by A and B.

Also ipc/shm.c does no sanity checks about which swap pages are in
flight. So the lock can't be removed until this is fixed as well.

Eric