On Fri, 20 Apr 2007 16:04:04 +0100 Andy Whitcroft <[EMAIL PROTECTED]> wrote:
>
> Having selected an area at the end of the inactive list, reclaim is
> attempted for all LRU pages within that contiguous area. Currently,
> any pages in this area found to still be active or referenced are
> rotated back to the active list as normal and the rest reclaimed.
> At low orders there is a reasonable likelyhood of finding contigious
> inactive areas for reclaim. However when reclaiming at higher order
> there is a very low chance all pages in the area being inactive,
> unreferenced and therefore reclaimable.
>
> This patch modifies behaviour when reclaiming at higher order
> (order >= 4). All LRU pages within the target area are reclaimed,
> including both active and recently referenced pages.
um, OK, I guess.
Should we use smaller values of 4 if PAGE_SIZE > 4k? I mean, users of the
page allocator usually request a number of bytes, not a number of pages.
Order 3 allocations on 64k pagesize will be far less common than on 4k
pagesize, no?
And is there a relationship between this magic 4 and the magic 3 in
__alloc_pages()? (Which has the same PAGE_SIZE problem, btw)
I must say that this is a pretty grotty-looking patch.
> [EMAIL PROTECTED]: additionally apply pressure to referenced paged]
> Signed-off-by: Andy Whitcroft <[EMAIL PROTECTED]>
> Acked-by: Mel Gorman <[EMAIL PROTECTED]>
> ---
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 466435f..e5e77fb 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -472,7 +472,7 @@ static unsigned long shrink_page_list(struct list_head
> *page_list,
>
> referenced = page_referenced(page, 1);
> /* In active use or really unfreeable? Activate it. */
> - if (referenced && page_mapping_inuse(page))
> + if (sc->order <= 3 && referenced && page_mapping_inuse(page))
The oft-occurring magic "3" needs a #define.
> @@ -599,6 +599,7 @@ keep:
> *
> * returns 0 on success, -ve errno on failure.
> */
> +#define ISOLATE_BOTH -1 /* Isolate both active and inactive
> pages. */
> static int __isolate_lru_page(struct page *page, int active)
> {
> int ret = -EINVAL;
> @@ -608,7 +609,8 @@ static int __isolate_lru_page(struct page *page, int
> active)
> * dealing with comparible boolean values. Take the logical not
> * of each.
> */
> - if (PageLRU(page) && (!PageActive(page) == !active)) {
> + if (PageLRU(page) && (active == ISOLATE_BOTH ||
> + (!PageActive(page) == !active))) {
So we have a nice enumerated value but we only half-use it: sometimes we
implicitly assume that ISOLATE_BOTH has a non-zero value, which rather
takes away from the whole point of creating ISOLATE_BOTH in the first
place.
Cleaner to do:
#define ISOLATE_INACTIVE 0
#define ISOLATE_ACTIVE 1
#define ISOLATE_BOTH 2
if (!PageLRU(page))
return; /* save a tabstop! */
if (active != ISOLATE_BOTH) {
if (PageActive(page) && active != ISOLATE_ACTIVE)
return;
if (!PageActive(page) && active != ISOLATE_INACTIVE)
return;
}
<isolate the page>
or some such. At present it is all very confused.
And the comment describing the `active' arg to __isolate_lru_page() needs
to be updated.
And the name `active' is now clearly inappropriate. It needs to be renamed
`mode' or something.
> ret = -EBUSY;
> if (likely(get_page_unless_zero(page))) {
> /*
> @@ -729,6 +731,26 @@ static unsigned long isolate_lru_pages(unsigned long
> nr_to_scan,
> }
>
> /*
> + * deactivate_pages() is a helper for shrink_active_list(), it deactivates
> + * all active pages on the passed list.
> + */
> +static unsigned long deactivate_pages(struct list_head *page_list)
The phrase "deactivate a page" normally means "move it from the active list
to the inactive list". But that isn't what this function does. Something
like clear_active_flags(), maybe?
> +{
> + int nr_active = 0;
> + struct list_head *entry;
> +
> + list_for_each(entry, page_list) {
> + struct page *page = list_entry(entry, struct page, lru);
list_for_each_entry()?
> + if (PageActive(page)) {
> + ClearPageActive(page);
> + nr_active++;
> + }
> + }
> +
> + return nr_active;
> +}
-
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/
