Re: [PATCH 1/4] Add kswapd descriptor.

[KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>]

On Tue, 30 Nov 2010 12:17:33 -0800
Ying Han <yinghan@google.com> wrote:

> > At the 1st look, this just seem to increase the size of changes....
> >
> > IMHO, implementing background-reclaim-for-memcg is cleaner than reusing kswapd..
> > kswapd has tons of unnecessary checks.
> 
> Sorry I am not aware of "background-reclaim-for-memcg", can you
> specify little bit more? Also,
> the unnecessary checks here refers to the kswapd() or balance_pgdat()?
> If the latter one, the
> logic is not being shared at all included in patch/3.
> 
Yes, now I read patch/3 and I'm sorry to say that.


Some nits.

At 1st. I just coudln't undestand idea of array of kswapd descriptor.
Hmm, dynamic allocation isn't possible ? as

==
struct kswapd_param {
	pg_data_t	*pgdat;
	struct mem_cgroup *memcg;
	struct wait_queue *waitq;
};


int kswapd_run(int nid, struct mem_cgroup *memcg)
{
	struct kswapd_param *param;

	param = kzalloc(); /* freed by kswapd */

	if (!memcg) { /* per-node kswapd */
		param->pgdat = NODE_DATA(nid);
		if (param->pgdat->kswapd)
			return;
		pgdat->kswapd = kthread_run(param);
		..../* fatal error check */
		return;
	}
		
	/* per-memcg kswapd */
	kthread_run(param);
}
==

Secondaly, I think some macro is necessary.

How about
==
#define is_node_kswapd(param)	(!param->memcg)

int kswapd(void *p)
{
	struct kswapd_param *param = p;

	if (is_node_kswapd(param))
		param->waitq = &param->pgdat->kswapd_wait;
	else
		param->waitq = mem_cgroup_get_kswapd_waitq(param->memcg);
		/* Here, we can notify the memcg which thread is for yours. */


or some ?

I think a macro like scanning_global_lru() is necessary.


Thanks,
-Kame

--
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>