[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH] block, migration: Use qemu_madvise
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH] block, migration: Use qemu_madvise inplace of madvise |
Date: |
Fri, 17 Feb 2017 13:31:44 +0100 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Fri 17 Feb 2017 12:30:28 PM CET, Pankaj Gupta wrote:
>> > To maintain consistency at all the places use qemu_madvise wrapper
>> > inplace of madvise call.
>>
>> > - madvise((uint8_t *) t + offset, length, MADV_DONTNEED);
>> > + qemu_madvise((uint8_t *) t + offset, length, QEMU_MADV_DONTNEED);
>>
>> Those two calls are not equivalent, please see commit
>> 2f2c8d6b371cfc6689affb0b7e for an explanation.
> I don't understand why '2f2c8d6b371cfc6689affb0b7e' explicitly changed
> for :"#ifdef CONFIG_LINUX" I think its better to write generic
> function maybe in a wrapper then to conditionally set something at
> different places.
The problem with qemu_madvise(QEMU_MADV_DONTNEED) is that it can mean
different things depending on the platform:
posix_madvise(POSIX_MADV_DONTNEED)
madvise(MADV_DONTNEED)
The first call is standard but it doesn't do what we need, so we cannot
use it.
The second call -- madvise(MADV_DONTNEED) -- is not standard, and it
doesn't do the same in all platforms. The only platform in which it does
what we need is Linux, hence the #ifdef CONFIG_LINUX and #if
defined(__linux__) that you see in the code.
I agree with David's comment that maybe it's better to remove
QEMU_MADV_DONTNEED altogether since it's not reliable.
Berto