Messages in this thread | | | Date | Tue, 28 Feb 2012 10:16:14 +0400 | From | Konstantin Khlebnikov <> | Subject | Re: [PATCH v3 07/21] mm: add lruvec->pages_count |
| |
KAMEZAWA Hiroyuki wrote: > On Thu, 23 Feb 2012 17:52:08 +0400 > Konstantin Khlebnikov<khlebnikov@openvz.org> wrote: > >> Move lru pages counter from mem_cgroup_per_zone->count[] to lruvec->pages_count[] >> >> Account pages in all lruvecs, incuding root, >> this isn't a huge overhead, but it greatly simplifies all code. >> >> Redundant page_lruvec() calls will be optimized in further patches. >> >> Signed-off-by: Konstantin Khlebnikov<khlebnikov@openvz.org> > > Hmm, I like this but..a question below. > >> --- >> include/linux/memcontrol.h | 29 -------------- >> include/linux/mm_inline.h | 15 +++++-- >> include/linux/mmzone.h | 1 >> mm/memcontrol.c | 93 +------------------------------------------- >> mm/swap.c | 7 +-- >> mm/vmscan.c | 25 +++++++++--- >> 6 files changed, 34 insertions(+), 136 deletions(-) >> >> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h >> index 4822d53..b9d555b 100644 >> --- a/include/linux/memcontrol.h >> +++ b/include/linux/memcontrol.h >> @@ -63,12 +63,6 @@ extern int mem_cgroup_cache_charge(struct page *page, struct mm_struct *mm, >> gfp_t gfp_mask); >> >> struct lruvec *mem_cgroup_zone_lruvec(struct zone *, struct mem_cgroup *); >> -struct lruvec *mem_cgroup_lru_add_list(struct zone *, struct page *, >> - enum lru_list); >> -void mem_cgroup_lru_del_list(struct page *, enum lru_list); >> -void mem_cgroup_lru_del(struct page *); >> -struct lruvec *mem_cgroup_lru_move_lists(struct zone *, struct page *, >> - enum lru_list, enum lru_list); >> >> /* For coalescing uncharge for reducing memcg' overhead*/ >> extern void mem_cgroup_uncharge_start(void); >> @@ -212,29 +206,6 @@ static inline struct lruvec *mem_cgroup_zone_lruvec(struct zone *zone, >> return&zone->lruvec; >> } >> >> -static inline struct lruvec *mem_cgroup_lru_add_list(struct zone *zone, >> - struct page *page, >> - enum lru_list lru) >> -{ >> - return&zone->lruvec; >> -} >> - >> -static inline void mem_cgroup_lru_del_list(struct page *page, enum lru_list lru) >> -{ >> -} >> - >> -static inline void mem_cgroup_lru_del(struct page *page) >> -{ >> -} >> - >> -static inline struct lruvec *mem_cgroup_lru_move_lists(struct zone *zone, >> - struct page *page, >> - enum lru_list from, >> - enum lru_list to) >> -{ >> - return&zone->lruvec; >> -} >> - >> static inline struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page) >> { >> return NULL; >> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h >> index 8415596..daa3d15 100644 >> --- a/include/linux/mm_inline.h >> +++ b/include/linux/mm_inline.h >> @@ -24,19 +24,24 @@ static inline int page_is_file_cache(struct page *page) >> static inline void >> add_page_to_lru_list(struct zone *zone, struct page *page, enum lru_list lru) >> { >> - struct lruvec *lruvec; >> + struct lruvec *lruvec = page_lruvec(page); >> + int numpages = hpage_nr_pages(page); >> >> - lruvec = mem_cgroup_lru_add_list(zone, page, lru); >> list_add(&page->lru,&lruvec->pages_lru[lru]); >> - __mod_zone_page_state(zone, NR_LRU_BASE + lru, hpage_nr_pages(page)); >> + lruvec->pages_count[lru] += numpages; >> + __mod_zone_page_state(zone, NR_LRU_BASE + lru, numpages); >> } >> >> static inline void >> del_page_from_lru_list(struct zone *zone, struct page *page, enum lru_list lru) >> { >> - mem_cgroup_lru_del_list(page, lru); >> + struct lruvec *lruvec = page_lruvec(page); >> + int numpages = hpage_nr_pages(page); >> + >> list_del(&page->lru); >> - __mod_zone_page_state(zone, NR_LRU_BASE + lru, -hpage_nr_pages(page)); >> + lruvec->pages_count[lru] -= numpages; >> + VM_BUG_ON((long)lruvec->pages_count[lru]< 0); >> + __mod_zone_page_state(zone, NR_LRU_BASE + lru, -numpages); >> } >> >> /** >> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h >> index be8873a..69b0f31 100644 >> --- a/include/linux/mmzone.h >> +++ b/include/linux/mmzone.h >> @@ -298,6 +298,7 @@ struct zone_reclaim_stat { >> >> struct lruvec { >> struct list_head pages_lru[NR_LRU_LISTS]; >> + unsigned long pages_count[NR_LRU_LISTS]; > > In this time, you don't put the objects under #ifdef...why ?
It will make the code much uglier and does not speed up it at all.
> > How do you handle duplication "the number of pages in LRU" of zone->vm_stat and this ?
I don't think this is totally bad, vmstat usually has per-cpu drift, these numbers will be exact.
> > Thanks, > -Kame >
| |