lkml.org 
[lkml]   [2006]   [Jan]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [patch 3/3] mm: PageActive no testset
Hi Nick,

On Wed, Jan 18, 2006 at 11:40:58AM +0100, Nick Piggin wrote:
> PG_active is protected by zone->lru_lock, it does not need TestSet/TestClear
> operations.

page->flags bits (including PG_active and PG_lru bits) are touched by
several codepaths which do not hold zone->lru_lock.

AFAICT zone->lru_lock guards access to the LRU list, and no more than
that.

Moreover, what about consistency of the rest of page->flags bits?

PPC for example implements test_and_set_bit() with:

lwarx reg, addr (load and create reservation for 32-bit addr)
or reg, BITOP_MASK(nr)
stwcx reg, addr (store word upon reservation validation, otherwise loop)

If you don't use atomic operations on page->flags, unrelated bits other
than that you're working with can have their updates lost, given that
in reality page->flags is not protected by the lru_lock.

For example:

/*
* shrink_list adds the number of reclaimed pages to sc->nr_reclaimed
*/
static int shrink_list(struct list_head *page_list, struct scan_control *sc)
{
...
BUG_ON(PageActive(page));
...
activate_locked:
SetPageActive(page);
pgactivate++;
keep_locked:
unlock_page(page);
keep:
list_add(&page->lru, &ret_pages);
BUG_ON(PageLRU(page));
}

And recently:

#ifdef CONFIG_MIGRATION
static inline void move_to_lru(struct page *page)
{
list_del(&page->lru);
if (PageActive(page)) {
/*
* lru_cache_add_active checks that
* the PG_active bit is off.
*/
ClearPageActive(page);
lru_cache_add_active(page);

Not relying on zone->lru_lock allows interesting optimizations
such as moving active/inactive pgflag bit setting from inside
__pagevec_lru_add/__pagevec_lru_add_active to the caller, and merging
the two.

Comments?


> Signed-off-by: Nick Piggin <npiggin@suse.de>
>
> Index: linux-2.6/mm/vmscan.c
> ===================================================================
> --- linux-2.6.orig/mm/vmscan.c
> +++ linux-2.6/mm/vmscan.c
> @@ -997,8 +997,9 @@ refill_inactive_zone(struct zone *zone,
> prefetchw_prev_lru_page(page, &l_inactive, flags);
> BUG_ON(PageLRU(page));
> SetPageLRU(page);
> - if (!TestClearPageActive(page))
> - BUG();
> + BUG_ON(!PageActive(page));
> + ClearPageActive(page);
> +
> list_move(&page->lru, &zone->inactive_list);
> pgmoved++;
> if (!pagevec_add(&pvec, page)) {
> Index: linux-2.6/mm/swap.c
> ===================================================================
> --- linux-2.6.orig/mm/swap.c
> +++ linux-2.6/mm/swap.c
> @@ -356,8 +356,8 @@ void __pagevec_lru_add_active(struct pag
> }
> BUG_ON(PageLRU(page));
> SetPageLRU(page);
> - if (TestSetPageActive(page))
> - BUG();
> + BUG_ON(PageActive(page));
> + SetPageActive(page);
> add_page_to_active_list(zone, page);
> }
> if (zone)
> Index: linux-2.6/include/linux/page-flags.h
> ===================================================================
> --- linux-2.6.orig/include/linux/page-flags.h
> +++ linux-2.6/include/linux/page-flags.h
> @@ -251,8 +251,6 @@ extern void __mod_page_state_offset(unsi
> #define PageActive(page) test_bit(PG_active, &(page)->flags)
> #define SetPageActive(page) set_bit(PG_active, &(page)->flags)
> #define ClearPageActive(page) clear_bit(PG_active, &(page)->flags)
> -#define TestClearPageActive(page) test_and_clear_bit(PG_active, &(page)->flags)
> -#define TestSetPageActive(page) test_and_set_bit(PG_active, &(page)->flags)
>
> #define PageSlab(page) test_bit(PG_slab, &(page)->flags)
> #define SetPageSlab(page) set_bit(PG_slab, &(page)->flags)
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-01-18 17:16    [W:0.098 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site