From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mel Gorman <mel@csn.ul.ie>
Subject: Re: [v7 7/8] mm: only IPI CPUs to drain local pages if they exist
Date: Mon, 30 Jan 2012 15:44:13 +0000
Message-ID: <20120130154413.GT25268@csn.ul.ie>
References: <1327572121-13673-1-git-send-email-gilad@benyossef.com>
 <1327572121-13673-8-git-send-email-gilad@benyossef.com>
 <20120130145900.GR25268@csn.ul.ie>
 <CAOtvUMcshnvQs4q4ySbtySWv_qHeEnHiD4USBSiOLGFNHSwzUw@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=iso-8859-15
Content-Transfer-Encoding: quoted-printable
Cc: linux-kernel@vger.kernel.org,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Christoph Lameter <cl@linux.com>,
	Chris Metcalf <cmetcalf@tilera.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Russell King <linux@arm.linux.org.uk>, linux-mm@kvack.org,
	Pekka Enberg <penberg@kernel.org>, Matt Mackall <mpm@selenic.com>,
	Sasha Levin <levinsasha928@gmail.com>,
	Rik van Riel <riel@redhat.com>, Andi Kleen <andi@firstfloor.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org, Avi Kivity <avi@redhat.com>,
	Michal Nazarewicz <mina86@mina86.com>,
	Milton Miller <miltonm@bga.com>
To: Gilad Ben-Yossef <gilad@benyossef.com>
Return-path: <owner-linux-mm@kvack.org>
Content-Disposition: inline
In-Reply-To: <CAOtvUMcshnvQs4q4ySbtySWv_qHeEnHiD4USBSiOLGFNHSwzUw@mail.gmail.com>
Sender: owner-linux-mm@kvack.org
List-Id: linux-fsdevel.vger.kernel.org

On Mon, Jan 30, 2012 at 05:14:37PM +0200, Gilad Ben-Yossef wrote:
> >> + =A0 =A0 for_each_online_cpu(cpu) {
> >> + =A0 =A0 =A0 =A0 =A0 =A0 bool has_pcps =3D false;
> >> + =A0 =A0 =A0 =A0 =A0 =A0 for_each_populated_zone(zone) {
> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pcp =3D per_cpu_ptr(zone->=
pageset, cpu);
> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (pcp->pcp.count) {
> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 has_pcps =3D=
 true;
> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> >> + =A0 =A0 =A0 =A0 =A0 =A0 }
> >> + =A0 =A0 =A0 =A0 =A0 =A0 if (has_pcps)
> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cpumask_set_cpu(cpu, &cpus=
_with_pcps);
> >> + =A0 =A0 =A0 =A0 =A0 =A0 else
> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cpumask_clear_cpu(cpu, &cp=
us_with_pcps);
> >> + =A0 =A0 }
> >
> > Lets take two CPUs running this code at the same time. CPU 1 has per-=
cpu
> > pages in all zones. CPU 2 has no per-cpu pages in any zone. If both r=
un
> > at the same time, CPU 2 can be clearing the mask for CPU 1 before it =
has
> > had a chance to send the IPI. This means we'll miss sending IPIs to C=
PUs
> > that we intended to.
>=20
> I'm confused. You seem to be assuming that each CPU is looking at its o=
wn pcps
> only (per zone).

/me slaps self

I was assuming exactly this.

> Assuming no change in the state of the pcps when both CPUs
> run this code at the same time, both of them should mark the bit for
> their respective
> CPUs the same, unless one of them raced and managed to send the IPI to =
clear
> pcps from the other, at which point you might see one of them send a
> spurious IPI
> to drains pcps that have been drained - but that isn't bad.
>=20

Indeed, the race is tiny and the consequences are not important.

> At least, that is what I meant the code to do and what I believe it
> does. What have I
> missed?
>=20

Nothing, the problem was on my side. Sorry for the noise.

--=20
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter=
.ca/
Don't email: <a href=3Dmailto:"dont@kvack.org"> email@kvack.org </a>