[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LI
From: |
Eduardo Habkost |
Subject: |
Re: [PATCH v2 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX |
Date: |
Mon, 16 Mar 2020 13:30:39 -0400 |
On Sun, Mar 15, 2020 at 05:15:46PM -0400, Michael S. Tsirkin wrote:
> On Sun, Mar 15, 2020 at 11:45:59AM -0400, Eduardo Habkost wrote:
> > On Wed, Mar 11, 2020 at 07:23:42PM -0400, Eduardo Habkost wrote:
> > > The CONFIG_LINUX check at the top of mmap-alloc.c never worked
> > > because it was done before including osdep.h.
> > >
> > > This means MAP_SYNC and MAP_SHARED_VALIDATE would always be set
> > > to 0 at the beginning of the file. Luckily, this didn't break
> > > when using recent glibc versions (2.28+), because those macros
> > > were redefined by glibc headers.
> > >
> > > Move the CONFIG_LINUX check after the main include lines, so the
> > > CONFIG_LINUX check works and we actually include <linux/mman.h>.
> > > This will make MAP_SYNC and MAP_SHARED_VALIDATE available even if
> > > the host has an older glibc version.
>
> Wait a second, MAP_SHARED_VALIDATE is from
> linux-headers/linux/mman.h - it's available on all architectures.
Yes, but both MAP_SYNC and MAP_SHARED_VALIDATE aren't available
if the host is not Linux.
>
> > >
> > > Reported-by: Jingqi Liu <address@hidden>
> > > Reviewed-by: Michael S. Tsirkin <address@hidden>
> > > Signed-off-by: Eduardo Habkost <address@hidden>
> > > ---
> > > Changes v1 -> v2:
> > > * (none)
> > > ---
> > > util/mmap-alloc.c | 7 +++----
> > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
> > > index 27dcccd8ec..7c2ce98eb0 100644
> > > --- a/util/mmap-alloc.c
> > > +++ b/util/mmap-alloc.c
> > > @@ -9,6 +9,9 @@
> > > * This work is licensed under the terms of the GNU GPL, version 2 or
> > > * later. See the COPYING file in the top-level directory.
> > > */
> > > +#include "qemu/osdep.h"
> > > +#include "qemu/mmap-alloc.h"
> > > +#include "qemu/host-utils.h"
> > >
> > > #ifdef CONFIG_LINUX
> > > #include <linux/mman.h>
> >
> > This breaks the build on mips, because mips doesn't have MAP_SYNC
> > defined at linux/mman.h:
> >
> > https://app.shippable.com/github/ehabkost/qemu-hacks/runs/9/9/console
>
>
> Oops. But that in fact means it's currently building on mips but not
> working correctly there! MAP_SHARED_VALIDATE 0x0 is especially
> problematic. I'm unsure what's the right thing to do is,
> I guess as a first step we can go back and device MAP_SYNC to 0,
Defining MAP_SYNC to 0 on MIPS would restore the existing
behavior, so it seems like a reasonable step to fix the build
failure. But not even printing a warning when the host doesn't
have MAP_SYNC (the existing behavior on MIPS and non-Linux) seems
wrong.
--
Eduardo
- [PATCH v2 0/2] Fix MAP_SYNC support when host has older glibc version, Eduardo Habkost, 2020/03/11
- [PATCH v2 1/2] Use -isystem for linux-headers dir, Eduardo Habkost, 2020/03/11
- [PATCH v2 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX, Eduardo Habkost, 2020/03/11
- Re: [PATCH v2 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX, Michael S. Tsirkin, 2020/03/12
- Re: [PATCH v2 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX, Eduardo Habkost, 2020/03/15
- Re: [PATCH v2 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX, Michael S. Tsirkin, 2020/03/15
- Re: [PATCH v2 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX,
Eduardo Habkost <=
- Re: [PATCH v2 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX, Peter Maydell, 2020/03/16
- Re: [PATCH v2 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX, Eduardo Habkost, 2020/03/16
- Re: [PATCH v2 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX, Michael S. Tsirkin, 2020/03/16
- Re: [PATCH v2 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX, Peter Maydell, 2020/03/16
- Re: [PATCH v2 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX, Michael S. Tsirkin, 2020/03/16
Re: [PATCH v2 0/2] Fix MAP_SYNC support when host has older glibc version, no-reply, 2020/03/11
Re: [PATCH v2 0/2] Fix MAP_SYNC support when host has older glibc version, Paolo Bonzini, 2020/03/14