Re: [PATCH] vm - swap_prefetch-11

[Jonathan Corbet]

Hi, Con,

> This patch implements swap prefetching when the vm is relatively idle and
> there is free ram available. 

I'm having a look at it now (better late than never...), and a couple of
questions come to mind...

The more general of the two is: would it make sense to somehow merge
your swapped_entry data structure with Rik's page-remembering scheme for
CLOCK-PRO?  Assumed that both are someday destined for inclusion, it
seems it would make sense to add just one "remember info about swapped
pages" data structure, rather than two.

Second question:

> +++ linux-2.6.13-sp/include/linux/fs.h	2005-09-23 16:57:03.000000000 +1000
> @@ -340,6 +340,8 @@ struct address_space {
>  	struct inode		*host;		/* owner: inode, block_device */
>  	struct radix_tree_root	page_tree;	/* radix tree of all pages */
>  	rwlock_t		tree_lock;	/* and rwlock protecting it */
> +	struct radix_tree_root	swap_tree;	/* radix tree of swapped pages */
> +	struct list_head	swapped_pages;	/* list of swapped pages */

It looks like you are adding these fields to every address_space
structure in the system - and there can be a fair number of those.  But
further down, when it comes time to remember a swapped page:

> +void add_to_swapped_list(unsigned long index)
> +{
> +	struct swapped_entry_t *entry;
> +	struct address_space *mapping = &swapper_space;

You're only actually remembering pages associated with a single address
space.  

Do you anticipate adding prefetching from other address spaces as well?
If not, it might be worth putting these structures somewhere else to
avoid bloating the address_space structure.

...or am I missing something again...?

Thanks,

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