From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: regression from c/s 22071:c5aed2e049bc (ept: Put locks around ept_get_entry) ? Date: Mon, 20 Dec 2010 16:24:23 +0000 Message-ID: References: <20101217111548.GQ9912@whitby.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20101217111548.GQ9912@whitby.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Tim Deegan Cc: "xen-devel@lists.xensource.com" , Keir Fraser , Jan Beulich List-Id: xen-devel@lists.xenproject.org Sorry for the delay in responding; I caught a bad cold, and was in no state to comment on making lockless races benign. :-) On Fri, Dec 17, 2010 at 11:15 AM, Tim Deegan wrote: > George, I think the underlying logic is still racy - the > check-and-populate function is checking a pointer that was found outside > the lock. =A0It needs to start again from the beginning to be safe, which > probably means just dropping the "check" part and letting the > p2m_pod_demand_populate handle lost races. =A0Also, why doesn't > ept_get_entry use a single read at the lowest level? My main goal when I wrote this patch was to fix a bug our testing found that was going to slip a release, and then move on to other pressing issues. So I'm sure there's more raciness to clean up in here that just isn't triggered generally. I think you're right, dropping the check and doing it in p2m_pod_demand_populate() is probably the Right Thing to do. I've got that stack of patches to deal with in January, I'll work on it then. -George Dunlap