From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dushyant Behl Subject: Re: [PATCH v2 1/3] [GSOC14] refactored mempaging code from xenpaging to libxc. Date: Fri, 27 Jun 2014 16:09:34 +0530 Message-ID: References: <1402942851-12538-1-git-send-email-myselfdushyantbehl@gmail.com> <1402942851-12538-2-git-send-email-myselfdushyantbehl@gmail.com> <1403864836.25894.8.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1403864836.25894.8.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: David Scott , Stefano Stabellini , Andrew Cooper , Ian Jackson , xen-devel@lists.xen.org, Andres Lagar Cavilla List-Id: xen-devel@lists.xenproject.org >> This patch is part of the work done under the gsoc project - >> Lazy Restore Using Memory Paging. > > For future patches please can you omit this, it's not really relevant to > the commit history. If you really want to include it please put it below > the --- marker. Or just mention it in the 0/N mail. > >> >> This patch moves the code to initialize and teardown mempaging from xenpaging >> to libxc. The code refractored from xenpaging is the code which sets up paging, > > "refactored" > > >> initializes a shared ring and event channel to communicate with xen. This >> communication is done between the hypervisor and the dom0 tool which performs >> the activity of pager. Another routine is also added by this patch which will >> teardown the ring and mem paging setup. The xenpaging code is changed to use >> the newly created routines and is tested to properly build and work with this code. >> >> The refractoring is done so that any tool which will act as pager in > > "refactoring" > >> lazy restore or use memory paging can use a same routines to initialize mempaging. >> This refactoring will also allow any future (in-tree) tools to use mempaging. >> >> The refractored code in xc_mem_paging_ring_setup is to be compiled into > > and again. > > I'll fix these up on commit, which I'll do as soon as someone confirms > that... > >> + /* Now that the ring is set, remove it from the guest's physmap */ >> + if ( xc_domain_decrease_reservation_exact(xch, domain_id, 1, 0, &ring_pfn) ) > > ... the lack of a domain pause and/or clearing the ring content in this > function does not represent an issue similar to > http://xenbits.xen.org/xsa/advisory-99.html. > >> + /* Unbind the event channel. */ >> + rc = xc_evtchn_unbind(xce_handle, *port); >> + if ( rc != 0 ) >> + { >> + PERROR("Error unbinding event port"); >> + } > > We don't in general require {}'s around single line statements. > >> +/* >> + * The name is kept BUF_RING_SIZE, because the name RING_SIZE >> + * collides with the xen shared ring definitions in io/ring.h >> + */ > > This sort of comment generally belongs in the commit log. > > Neither of those last two are a blocker for applying though. Ian, Please don't apply the patch yet. I have found a very small yet big mistake with the patch. I'll send the correct patch along with these things updated correctly. Thanks and Regards, Dushyant Behl