From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <441E94FA.8070408@yahoo.com.au> Date: Mon, 20 Mar 2006 22:41:46 +1100 From: Nick Piggin MIME-Version: 1.0 Subject: Re: [PATCH][1/3] mm: swsusp shrink_all_memory tweaks References: <200603200231.50666.kernel@kolivas.org> In-Reply-To: <200603200231.50666.kernel@kolivas.org> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org Return-Path: To: Con Kolivas Cc: linux list , ck list , Andrew Morton , Rafael Wysocki , Pavel Machek , linux-mm@kvack.org List-ID: Con Kolivas wrote: > - > + > +#define for_each_priority_reverse(priority) \ > + for (priority = DEF_PRIORITY; \ > + priority >= 0; \ > + priority--) > + > /* > * This is the main entry point to direct page reclaim. > * > @@ -979,7 +1010,7 @@ unsigned long try_to_free_pages(struct z > lru_pages += zone->nr_active + zone->nr_inactive; > } > > - for (priority = DEF_PRIORITY; priority >= 0; priority--) { > + for_each_priority_reverse(priority) { > sc.nr_mapped = read_page_state(nr_mapped); > sc.nr_scanned = 0; > if (!priority) I still don't like this change. Apart from being harder to read in my opinion, I don't believe there is a precedent for "consolidating" simple for loops in the kernel, is there? More complex loops get helpers, but they're made part of the wider well-known kernel API. Why does for_each_priority_reverse blow up when you pass it an unsigned argument? What range has priority? What direction does the loop go in? (_reverse postfix doesn't tell me, because it is going from low->high priority so I would have thought that is going forward, or up) You had to look in two places each time you wanted to know the answers. -- SUSE Labs, Novell Inc. Send instant messages to your online friends http://au.messenger.yahoo.com -- 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: email@kvack.org