Re: [Qemu-devel] Purpose of memory-backend-file.discard-data

2018-04-17 Thread Michal Privoznik
On 04/16/2018 07:59 PM, Eduardo Habkost wrote:
> (CCing Zack Cornelius)
> 
> On Fri, Apr 13, 2018 at 08:21:26AM +0200, Michal Privoznik wrote:
>> Eduardo et al,
>>
>> I'm looking at 11ae6ed8affdd131e and I wanted to implement libvirt
>> support for that. But more I look at it less I understand it. My
>> understanding it is an optimization (although not very effective one
>> since madvise() is/should be immediately followed by munmap()). So any
>> application that is trying to keep track of guest memory  can stop doing
>> so as soon as it sees munmap(). Or does the optimization lies in fact
>> that madvise() is called sooner and thus the app can stop caring
>> slightly sooner?
> 
> munmap() or close() are not enough: it would still make the host
> kernel unnecessarily write data to backing storage.

Something like sync()? I though that mem access is always synchronous.
Or perhaps I don't understand why is kernel still touching data after
munmap().

>  The point of
> the madvise() call is to tell the kernel that data can be
> destroyed instead of being written back.
> 
> The original use case is described by Zack here:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg473538.html
> 
> 
>>
>> Also, I don't quite understand why is this configurable. What's the harm
>> in turning it on by default? Yesterday I've posted some patches to
>> libvirt list [1] (although I have to admit I am still not fully
>> convinced about the design) and they implement just this - whenever qemu
>> supports the feature libvirt turns it on.S
> 
> The option can destroy the data on the backing file, so QEMU can't enable it 
> by
> default.  libvirt can enable it as long as it knows it can destroy the data on
> the backing file.

Destroy data? Oh, okay then. I rather make it configurable. Although I'm
not quite sure about XML design yet.

Michal



Re: [Qemu-devel] Purpose of memory-backend-file.discard-data

2018-04-16 Thread Eduardo Habkost
(CCing Zack Cornelius)

On Fri, Apr 13, 2018 at 08:21:26AM +0200, Michal Privoznik wrote:
> Eduardo et al,
> 
> I'm looking at 11ae6ed8affdd131e and I wanted to implement libvirt
> support for that. But more I look at it less I understand it. My
> understanding it is an optimization (although not very effective one
> since madvise() is/should be immediately followed by munmap()). So any
> application that is trying to keep track of guest memory  can stop doing
> so as soon as it sees munmap(). Or does the optimization lies in fact
> that madvise() is called sooner and thus the app can stop caring
> slightly sooner?

munmap() or close() are not enough: it would still make the host
kernel unnecessarily write data to backing storage.  The point of
the madvise() call is to tell the kernel that data can be
destroyed instead of being written back.

The original use case is described by Zack here:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg473538.html


> 
> Also, I don't quite understand why is this configurable. What's the harm
> in turning it on by default? Yesterday I've posted some patches to
> libvirt list [1] (although I have to admit I am still not fully
> convinced about the design) and they implement just this - whenever qemu
> supports the feature libvirt turns it on.S

The option can destroy the data on the backing file, so QEMU can't enable it by
default.  libvirt can enable it as long as it knows it can destroy the data on
the backing file.

> 
> Michal
> 
> 1: https://www.redhat.com/archives/libvir-list/2018-April/msg01066.html

-- 
Eduardo