From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH] Don't track all memory when enabling log dirty to track vram Date: Wed, 19 Feb 2014 11:03:50 +0000 Message-ID: <53048F96.7010503@eu.citrix.com> References: <20140210080314.GA758@deinos.phlegethon.org> <20140211090202.GC92054@deinos.phlegethon.org> <20140211115553.GB97288@deinos.phlegethon.org> <52FA2C63020000780011B201@nat28.tlf.novell.com> <52FA480D.9040707@eu.citrix.com> <52FCE8BE.8050105@eu.citrix.com> <52FCF90F020000780011C29A@nat28.tlf.novell.com> <20140213162022.GE82703@deinos.phlegethon.org> <5301F000020000780011CCE0@nat28.tlf.novell.com> <53022209.1060005@eu.citrix.com> <5302332D020000780011CEF1@nat28.tlf.novell.com> <53033544.2000409@eu.citrix.com> <53047F74020000780011D8E4@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53047F74020000780011D8E4@nat28.tlf.novell.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: Jan Beulich , Yang Z Zhang Cc: "andrew.cooper3@citrix.com" , Tim Deegan , Xiantao Zhang , Donald D Dugger , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 02/19/2014 08:55 AM, Jan Beulich wrote: >>>> On 19.02.14 at 02:28, "Zhang, Yang Z" wrote: >> George Dunlap wrote on 2014-02-18: >>> On 02/18/2014 03:14 AM, Zhang, Yang Z wrote: >>> perhaps my original patch is better which will check >>> paging_mode_log_dirty(d) && log_global: >>> >>> It turns out that the reason I couldn't get a crash was because libxc >>> was actually paying attention to the -EINVAL return value, and >>> disabling and then re-enabling logdirty. That's what would happen >>> before your dirty vram patch, and that's what happens after. And >>> arguably, that's the correct behavior for any toolstack, given that the >> interface returns an error. >> >> Agree. >> >>> This patch would actually change the interface; if we check this in, >>> then if you enable logdirty when dirty vram tracking is enabled, you >>> *won't* get an error, and thus *won't* disable and re-enable logdirty mode. >>> So actually, this patch would be more disruptive. >>> >> Jan, do you have any comment? > This simplistic variant is just calling for problems. As was already > said elsewhere on this thread, we should simply do the mode change > properly: Track that a partial log-dirty mode is in use, and allow > switching to global log-dirty mode (converting all entries to R/O). I think Yang was asking you for your opinion on my suggestion that nothing actually needed to be done. Enabling full logdirty mode for migration when dirty vram tracking was enabled has *always* returned an error (or at least for a long time now), and *always* resulted in the toolstack disabling and re-enabling logdirty mode; Yang's patch doesn't change that at all. If you think that's an interface we need to improve in the future, we can put it on the list of improvements. But at this point it seems to me more like a nice-to-have. -George