[PATCH] mm:kswapd: clean up the kswapd

[PATCH] mm:kswapd: clean up the kswapd

[Chen Yucong]

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@gmail.com>
---
 mm/vmscan.c |   29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index a8ffe4e..b0a75d1 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3332,10 +3332,9 @@ static void kswapd_try_to_sleep(pg_data_t *pgdat, int order, int classzone_idx)
  */
 static int kswapd(void *p)
 {
+	bool balance_is_successful;
 	unsigned long order, new_order;
-	unsigned balanced_order;
 	int classzone_idx, new_classzone_idx;
-	int balanced_classzone_idx;
 	pg_data_t *pgdat = (pg_data_t*)p;
 	struct task_struct *tsk = current;
 
@@ -3366,9 +3365,7 @@ static int kswapd(void *p)
 	set_freezable();
 
 	order = new_order = 0;
-	balanced_order = 0;
 	classzone_idx = new_classzone_idx = pgdat->nr_zones - 1;
-	balanced_classzone_idx = classzone_idx;
 	for ( ; ; ) {
 		bool ret;
 
@@ -3377,24 +3374,32 @@ static int kswapd(void *p)
 		 * 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) {
+		balance_is_successful = false;
+		if (classzone_idx >= new_classzone_idx && order == new_order) {
+			/*
+			 * After the last balance_pgdat, if the `order' stays
+			 * constant and the scanned zones are not less than
+			 * specified by original classzone_idx, then the last
+			 * balance_pgdat was successful.
+			 */
 			new_order = pgdat->kswapd_max_order;
 			new_classzone_idx = pgdat->classzone_idx;
 			pgdat->kswapd_max_order =  0;
 			pgdat->classzone_idx = pgdat->nr_zones - 1;
+			balance_is_successful = true;
 		}
 
-		if (order < new_order || classzone_idx > new_classzone_idx) {
+		if (balance_is_successful && (order < new_order ||
+					classzone_idx > new_classzone_idx)) {
 			/*
 			 * Don't sleep if someone wants a larger 'order'
-			 * allocation or has tigher zone constraints
+			 * allocation or has tighter zone constraints on the
+			 * premise of the last balance_pgdat was successful.
 			 */
 			order = new_order;
 			classzone_idx = new_classzone_idx;
 		} else {
-			kswapd_try_to_sleep(pgdat, balanced_order,
-						balanced_classzone_idx);
+			kswapd_try_to_sleep(pgdat, order, classzone_idx);
 			order = pgdat->kswapd_max_order;
 			classzone_idx = pgdat->classzone_idx;
 			new_order = order;
@@ -3413,9 +3418,7 @@ static int kswapd(void *p)
 		 */
 		if (!ret) {
 			trace_mm_vmscan_kswapd_wake(pgdat->node_id, order);
-			balanced_classzone_idx = classzone_idx;
-			balanced_order = balance_pgdat(pgdat, order,
-						&balanced_classzone_idx);
+			order = balance_pgdat(pgdat, order, &classzone_idx);
 		}
 	}
 
-- 
1.7.10.4

--
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>

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

[Mel Gorman]

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@gmail.com>

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.

-- 
Mel Gorman
SUSE Labs

--
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>

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@gmail.com>
> 
> 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@gmail.com>
---
 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, 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>