[patch 10/16] give swapper_space a set_page_dirty a_op

[patch 10/16] give swapper_space a set_page_dirty a_op

[Andrew Morton]

Give swapper_space a ->set_page_dirty() address_space_operation.

So swapcache pages do not need special-casing in
set_page_dirty_buffers().



=====================================

--- 2.5.19/mm/swap_state.c~swap-set_page_dirty	Sat Jun  1 01:18:11 2002
+++ 2.5.19-akpm/mm/swap_state.c	Sat Jun  1 01:18:11 2002
@@ -48,9 +48,10 @@ static int swap_vm_writeback(struct page
 }
 
 static struct address_space_operations swap_aops = {
-	vm_writeback: swap_vm_writeback,
-	writepage: swap_writepage,
-	sync_page: block_sync_page,
+	vm_writeback:	swap_vm_writeback,
+	writepage:	swap_writepage,
+	sync_page:	block_sync_page,
+	set_page_dirty:	__set_page_dirty_nobuffers,
 };
 
 /*
--- 2.5.19/mm/page-writeback.c~swap-set_page_dirty	Sat Jun  1 01:18:11 2002
+++ 2.5.19-akpm/mm/page-writeback.c	Sat Jun  1 01:18:11 2002
@@ -497,7 +497,7 @@ int __set_page_dirty_buffers(struct page
 
 	spin_lock(&mapping->private_lock);
 
-	if (page_has_buffers(page) && !PageSwapCache(page)) {
+	if (page_has_buffers(page)) {
 		struct buffer_head *head = page_buffers(page);
 		struct buffer_head *bh = head;
 


-

Re: [patch 10/16] give swapper_space a set_page_dirty a_op

[Jeff Garzik]

On Sat, Jun 01, 2002 at 01:42:48AM -0700, Andrew Morton wrote:
> 
> 
> Give swapper_space a ->set_page_dirty() address_space_operation.

Remember that we don't need swapper_space at all?

All the underlying inodes have their own address spaces, and the
SWP_ENTRY tells us what we need to know, to find the underlying address
spaces.

swapper_space is just a master address space that overlays the
underlying multiple address spaces.  We can just look directly at the
underlying ones...

	Jeff

Re: [patch 10/16] give swapper_space a set_page_dirty a_op

[Andrew Morton]

Jeff Garzik wrote:
> 
> On Sat, Jun 01, 2002 at 01:42:48AM -0700, Andrew Morton wrote:
> >
> >
> > Give swapper_space a ->set_page_dirty() address_space_operation.
> 
> Remember that we don't need swapper_space at all?

swapper_space makes some sense...

> All the underlying inodes have their own address spaces, and the
> SWP_ENTRY tells us what we need to know, to find the underlying address
> spaces.
> 
> swapper_space is just a master address space that overlays the
> underlying multiple address spaces.  We can just look directly at the
> underlying ones...

Nope.  swapper_space is a single, standalone mapping which implements
its own (striped) I/O direct to block devices.  To locate the blockdev
it goes:

	swap_files[type]->f_dentry->d_inode->i_bdev

Swap never talks to d_inode->i_mapping.  If someone else is accessing that
device then their view of the device will be via bdinode->i_mapping and
is not coherent with swapper_space.

swapper_space can get at the underlying device via its own block
mapping function or via a swapfile's bmap() function.   But either
way, it goes direct to the blockdev via brw_page().

All those inodes and file*'s are just a way for swap to get directly
at the blockdev, which is a good thing for swap to be doing.

Yes, possibly we could stripe swap at the address_space level, and
switch anon pages in and out of the individual blockdev mappings.

But that's more complex, and means that swap would have to be able to
deal with external agents coming in and randomly locking its buffers
and pages (it'll oops at present if that happens, but it can't because
nobody can get at swapper_space's pages and buffers).

Plus the blockdev mapping uses PAGE_CACHE_SIZE pages, and swap
uses PAGE_SIZE pages.   I do think the design is OK as-is.

-