Re: [RFC][PATCH] fix swap entries is not reclaimed in proper wayfor memg v3.

[KAMEZAWA Hiroyuki]

On Sun, 26 Apr 2009 01:06:58 +0900
Daisuke Nishimura <d-nishimura@xxxxxxxxxxxxxxxxx> wrote:

> A few minor nitpicks :)
> 
> > > +static void memcg_fixup_stale_swapcache(struct work_struct *work)
> > > +{
> > > +	int pos = 0;
> > > +	swp_entry_t entry;
> > > +	struct page *page;
> > > +	int forget, ret;
> > > +
> > > +	while (ssc.num) {
> > > +		spin_lock(&ssc.lock);
> > > +		pos = find_next_bit(ssc.usemap, STALE_ENTS, pos);
> > > +		spin_unlock(&ssc.lock);
> > > +
> > > +		if (pos >= STALE_ENTS)
> > > +			break;
> > > +
> > > +		entry = ssc.ents[pos];
> > > +
> > > +		forget = 1;
> > > +		page = lookup_swap_cache(entry);
> I think using find_get_page() would be better.
> lookup_swap_cache() update swapcache_info.
> 

ok, and I have to add put_page() somewhere.

> > > +		if (page) {
> > > +			lock_page(page);
> > > +			ret = try_to_free_swap(page);
> > > +			/* If it's still under I/O, don't forget it */
> > > +			if (!ret && PageWriteback(page))
> > > +				forget = 0;
> > > +			unlock_page(page);
> > I think we need page_cache_release().
> > lookup_swap_cache() gets the page.
> > 
> > > +		}
> > > +		if (forget) {
> > > +			spin_lock(&ssc.lock);
> > > +			clear_bit(pos, ssc.usemap);
> > > +			ssc.num--;
> > > +			if (ssc.num < STALE_ENTS/2)
> > > +				ssc.congestion = 0;
> > > +			spin_unlock(&ssc.lock);
> > > +		}
> > > +		pos++;
> > > +	}
> > > +	if (ssc.num) /* schedule me again */
> > > +		schedule_delayed_work(&ssc.gc_work, HZ/10);
> We can use schedule_ssc_gc() here.
> (It should be defined before this, of course. And can be inlined.)
> 
Sure.

will soon post v2. (removing RFC)

Thanks,
-Kame
> > "if (ssc.congestion)" would be better ?
> > 
> > > +	return;
> > > +}
> > > +
> > 
> 
> Thanks,
> Daisuke Nishimura.
> 

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