From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:51887) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qjv38-00018l-AN for qemu-devel@nongnu.org; Thu, 21 Jul 2011 11:20:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QjuuK-0001gk-EU for qemu-devel@nongnu.org; Thu, 21 Jul 2011 11:11:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:22536) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QjuuK-0001gg-65 for qemu-devel@nongnu.org; Thu, 21 Jul 2011 11:11:24 -0400 Message-ID: <4E284197.8000903@redhat.com> Date: Thu, 21 Jul 2011 18:11:19 +0300 From: Avi Kivity MIME-Version: 1.0 References: <1311243679-18403-1-git-send-email-avi@redhat.com> <4E280195.5040003@siemens.com> <4E281609.7090809@redhat.com> <4E2816DE.5010102@siemens.com> <4E2817DA.4030505@redhat.com> <4E28211C.5060705@siemens.com> <4E282286.2050508@redhat.com> <4E2826E1.6090404@siemens.com> <4E282E8A.40606@redhat.com> <4E283866.7040601@siemens.com> <4E283A1A.5070908@redhat.com> <4E28404D.1080002@siemens.com> In-Reply-To: <4E28404D.1080002@siemens.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] memory: transaction API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: "qemu-devel@nongnu.org" , "kvm@vger.kernel.org" On 07/21/2011 06:05 PM, Jan Kiszka wrote: > > > > The point is _update() can only make changes for one region atomic, > > while _commit() is more general. You can sometimes batch all changes > > into a single container region, but sometimes it is clumsy, and > > sometimes impossible. > > > > Deletion and creation are needed because we can't update an alias' > > offset. I guess I can add that functionality. But it still isn't as > > general as _commit(). > > OK. What about providing _update wrappers? They could be implemented > internally by the memory API in terms of begin - remove region - change > region object - re-add region - end. That would avoid boilerplate code > on the user side and still keep the option to do open-coded transaction > also outside the core. It's pretty easy to do updates without resorting to transactions. If it's just some property, you simply call m_r_update_topology(). Things like priority need a bit more logic to keep the list in sorted order, but it's not really difficult. > > > >>> > >>>> I also think now that describing a memory region offline via a struct > >>>> and then passing that to an atomic add/del/update would be a more handy > >>>> and future-proof API than an increasing number set functions. > >>> > >>> Maybe. But it's not sufficient for atomic changes involving multiple > >>> regions. > >> > >> Right. The question is still if there are use cases where this matters > >> (ie. update frequencies comparable to graphic scenarios). > > > > Does even cirrus update this often? I would guess cirrus usually uses > > the linear framebuffer, no? > > Not sure how this mode is called, but when vram is mapped linearly into > the 0xa0000 range via two 32K banks, you get quite a few updates on > larger screen changes. > Ok. grub2 again? or another guest? > > > >> But such an IOCTL would resolve our problem with dropping a logged > >> region as well, right? > > > > Yes, if done right. Still we need to support older kernels. > > We will need workarounds like we have today, e.g. confining PAM memory > region fragmentation to certain patterns that known OSes require. Full, > correct support would remain the privilege of host kernels that allow to > combine multi-region updates to an atomic operation (+ returning or > preserving dirty logs). An atomic update + dirty log fetch can be emulated by temporarily freezing all vcpus. -- error compiling committee.c: too many arguments to function