Re: [PATCH -mm] vmscan: merge duplicate code in shrink_active_list()

Rik van Riel
2009-05-12T13:37:22Z

Wu Fengguang wrote:
> On Tue, May 12, 2009 at 03:26:33PM +0800, Minchan Kim wrote:
>> On Tue, 12 May 2009 10:53:19 +0800
>> Wu Fengguang <fengguang.wu@intel.com> wrote:
>>
>>> The "move pages to active list" and "move pages to inactive list"
>>> code blocks are mostly identical and can be served by a function.
>>>
>>> Thanks to Andrew Morton for pointing this out.
>>>
>>> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
>>> ---
>>> mm/vmscan.c | 84 ++++++++++++++++++++------------------------------
>>> 1 file changed, 35 insertions(+), 49 deletions(-)
>>>
>>> --- linux.orig/mm/vmscan.c
>>> +++ linux/mm/vmscan.c
>>> @@ -1225,6 +1225,36 @@ static inline void note_zone_scanning_pr
>>> * But we had to alter page->flags anyway.
>>> */
>>>
>>> +void move_active_pages_to_lru(enum lru_list lru, struct list_head *list)
>>> +{
>>> + unsigned long pgmoved = 0;
>>> +
>>> + while (!list_empty(&list)) {
>>> + page = lru_to_page(&list);
>>> + prefetchw_prev_lru_page(page, &list, flags);
>>> +
>>> + VM_BUG_ON(PageLRU(page));
>>> + SetPageLRU(page);
>>> +
>>> + VM_BUG_ON(!PageActive(page));
>>> + if (lru < LRU_ACTIVE)
>>> + ClearPageActive(page);
>> Arithmetic on the LRU list is not good code for redability, I think.
>> How about adding comment?
>>
>> if (lru < LRU_ACTIVE) /* In case of moving from active list to inactive */
>>
>> Ignore me if you think this is trivial.
>
> Good suggestion. Or this simple one: "we are de-activating"?

lru < LRU_ACTIVE will never be true for file pages,
either active or inactive.

--
All rights reversed.