Re: [PATCH] mm:kswapd: clean up the kswapd

[Chen Yucong]

On Mon, 2014-06-23 at 12:19 +0100, Mel Gorman wrote:
> On Mon, Jun 23, 2014 at 01:14:54PM +0800, Chen Yucong wrote:
> > According to the commit 215ddd66 (mm: vmscan: only read new_classzone_idx from
> > pgdat when reclaiming successfully) and the commit d2ebd0f6b (kswapd: avoid
> > unnecessary rebalance after an unsuccessful balancing), we can use a boolean
> > variable for replace balanced_* variables, which makes the kswapd more clarify.
> > 
> > Signed-off-by: Chen Yucong <slaoub@xxxxxxxxx>
> 
> I think this is just churning code for the sake of it. It's not any
> easier to understand as a result of the modification and does not appear
> to be a preparation for a follow-on patch that addresses a bug.
> 
Anyway, there are some palaces that still need to be cleaned in
`kswapd'.

thx!
cyc


Subject: [PATCH] mm:kswapd: clean up the kswapd

The type of variables(order, new_order, and balanced_order) has some
flaws.
According to the `order' argument for kswapd_try_to_sleep() and
balance_pgdat(),
they should be defined as `int' rather than `unsigned long' or
`unsigned'.
This patch also does minimal cleanup, which makes the kswapd more
clarify.

Signed-off-by: Chen Yucong <slaoub@xxxxxxxxx>
---
 mm/vmscan.c |   33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index a8ffe4e..1b2576d 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3332,8 +3332,8 @@ static void kswapd_try_to_sleep(pg_data_t *pgdat,
int order, int classzone_idx)
  */
 static int kswapd(void *p)
 {
-	unsigned long order, new_order;
-	unsigned balanced_order;
+	int order, new_order;
+	int balanced_order;
 	int classzone_idx, new_classzone_idx;
 	int balanced_classzone_idx;
 	pg_data_t *pgdat = (pg_data_t*)p;
@@ -3371,34 +3371,37 @@ static int kswapd(void *p)
 	balanced_classzone_idx = classzone_idx;
 	for ( ; ; ) {
 		bool ret;
+		bool sleep = true;
 
 		/*
 		 * If the last balance_pgdat was unsuccessful it's unlikely a
 		 * new request of a similar or harder type will succeed soon
 		 * so consider going to sleep on the basis we reclaimed at
 		 */
-		if (balanced_classzone_idx >= new_classzone_idx &&
-					balanced_order == new_order) {
+		if (balanced_classzone_idx >= classzone_idx &&
+					balanced_order == order) {
 			new_order = pgdat->kswapd_max_order;
 			new_classzone_idx = pgdat->classzone_idx;
-			pgdat->kswapd_max_order =  0;
+			pgdat->kswapd_max_order = 0;
 			pgdat->classzone_idx = pgdat->nr_zones - 1;
+
+			if (order < new_order ||
+					classzone_idx > new_classzone_idx) {
+				/*
+				 * Don't sleep if someone wants a larger 'order'
+				 * allocation or has tighter zone constraints
+				 */
+				order = new_order;
+				classzone_idx = new_classzone_idx;
+				sleep = false;
+			}
 		}
 
-		if (order < new_order || classzone_idx > new_classzone_idx) {
-			/*
-			 * Don't sleep if someone wants a larger 'order'
-			 * allocation or has tigher zone constraints
-			 */
-			order = new_order;
-			classzone_idx = new_classzone_idx;
-		} else {
+		if (sleep) {
 			kswapd_try_to_sleep(pgdat, balanced_order,
 						balanced_classzone_idx);
 			order = pgdat->kswapd_max_order;
 			classzone_idx = pgdat->classzone_idx;
-			new_order = order;
-			new_classzone_idx = classzone_idx;
 			pgdat->kswapd_max_order = 0;
 			pgdat->classzone_idx = pgdat->nr_zones - 1;
 		}
-- 
1.7.10.4

  

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/