linux-kernel - Re: [RFC][PATCH] fix swap entries is not reclaimed in proper way for memg v3.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20090427163930.50604bf9.kamezawa.hiroyu@jp.fujitsu.com>
Date:	Mon, 27 Apr 2009 16:39:30 +0900
From:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To:	nishimura@....nes.nec.co.jp
Cc:	Daisuke Nishimura <d-nishimura@....biglobe.ne.jp>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"balbir@...ux.vnet.ibm.com" <balbir@...ux.vnet.ibm.com>,
	"hugh@...itas.com" <hugh@...itas.com>
Subject: Re: [RFC][PATCH] fix swap entries is not reclaimed in proper way
 for memg v3.

On Sun, 26 Apr 2009 01:06:58 +0900
Daisuke Nishimura <d-nishimura@....biglobe.ne.jp> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/