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, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>