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
