Re: [PATCH] Fix shared memory mapping

[Rongqing Li <rongqing.li@xxxxxxxxxxxxx>]

On 05/21/2013 04:47 AM, Steven Dake wrote:
> On 05/18/2013 06:00 PM, rongqing.li@xxxxxxxxxxxxx wrote:
> > From: "Roy.Li" <rongqing.li@xxxxxxxxxxxxx>
> >
> > On a number of arches, any fixed and shared mmap() mapping address
> > must be
> > aligned to 16k, or MIPS cpu askes that the address must be aligned to not
> > violate cache aliasing constraints.
>
> Roy,
>
> Just as a note, the reason zcb was introduced into the API was to
> improve performance.  I believe later we determined the zcb version of
> cpg was slower then just using cpg_mcast, but we have a strong desire to
> maintain backwards API compatibility.
>
> I would recommend just avoiding the zcb api calls all-together.
>
> If performance is your goal, take a look at 2.y.z series where
> performance was a major objective of Corosync.  The CPG API is
> significantly faster for many message sizes, small ones in particular,
> and has less CPU utilization for all message sizes which may be
> desirable for your target architecture.
>
> Regards
> -steve

Thanks for your reply, performance is not my goal.
My goal is to make it be able to work on MIPS arch.

If first mmap is MAP_ANONYMOUS, second mmap with
MAP_FIXED | MAP_SHARED will fail on MIPS arch.

similar issue and fixes is on libqb, we can refer it.
https://github.com/asalkeld/libqb/blob/master/lib/unix.c#L160

Thanks.

-Roy



> > If we create a shared memory like below:
> >
> >          addr_orig = mmap (NULL, bytes, PROT_NONE,
> >                            MAP_ANONYMOUS| MAP_PRIVATE, -1, 0);
> >
> >          addr = mmap (addr_orig, bytes, PROT_READ | PROT_WRITE,
> >                  MAP_FIXED | MAP_SHARED, fd, 0);
> >
> > If the first mmap() upper is not shared then the first mmap() will
> > succeed
> > because these restrictions do not apply to private mappings. The second
> > mmap() wants a shared memory mapping but the address returned by the
> > first
> > one is only page-aligned and not aligned to 16k, or violate cache
> > aliasing
> > constraints on MIPS cpu, will lead to second mmap failure.
> >
> > Signed-off-by: Roy.Li <rongqing.li@xxxxxxxxxxxxx>
> > ---
> >   configure.ac |   25 +++++++++++++++++++++++++
> >   exec/cpg.c   |   13 +++++++++----
> >   lib/cpg.c    |   10 ++++++++--
> >   3 files changed, 42 insertions(+), 6 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 523d2cf..d3a9c25 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -647,6 +647,31 @@ else
> >     AC_SUBST(VERSCRIPT_LDFLAGS, [""])
> >   fi
> > +arch_force_shmlba=no
> > +AC_MSG_CHECKING([for architecture in ${host_cpu}])
> > +case $host_cpu in
> > +    sparc*)
> > +        AC_MSG_RESULT([${host_cpu}])
> > +        arch_force_shmlba=yes
> > +        ;;
> > +    arm*)
> > +        AC_MSG_RESULT([${host_cpu}])
> > +        arch_force_shmlba=yes
> > +        ;;
> > +    mips*)
> > +        AC_MSG_RESULT([${host_cpu}])
> > +        arch_force_shmlba=yes
> > +        ;;
> > +    *)
> > +        AC_MSG_RESULT([${host_cpu}])
> > +    ;;
> > +esac
> > +
> > +if test $arch_force_shmlba = yes; then
> > +    AC_DEFINE_UNQUOTED([FORCE_SHM_ALIGN], [1],
> > +           [shared and fixed mmap must align on cache alias or 16k])
> > +fi
> > +
> >   # define global include dirs
> >   INCLUDE_DIRS="$INCLUDE_DIRS -I\$(top_builddir)/include
> > -I\$(top_srcdir)/include"
> >   INCLUDE_DIRS="$INCLUDE_DIRS -I\$(top_builddir)/include/corosync
> > -I\$(top_srcdir)/include/corosync"
> > diff --git a/exec/cpg.c b/exec/cpg.c
> > index 695f311..04260e0 100644
> > --- a/exec/cpg.c
> > +++ b/exec/cpg.c
> > @@ -1606,6 +1606,7 @@ memory_map (
> >       void *addr_orig;
> >       void *addr;
> >       int32_t res;
> > +    int flags = MAP_ANONYMOUS;
> >       fd = open (path, O_RDWR, 0600);
> > @@ -1620,15 +1621,19 @@ memory_map (
> >           goto error_close_unlink;
> >       }
> > -    addr_orig = mmap (NULL, bytes, PROT_NONE,
> > -        MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> > +#ifdef FORCE_SHM_ALIGN
> > +    flags |= MAP_SHARED;
> > +#else
> > +    flags |= MAP_PRIVATE;
> > +#endif /* FORCE_SHM_ALIGN */
> > +
> > +    addr_orig = mmap (NULL, bytes, PROT_NONE, flags, -1, 0);
> >       if (addr_orig == MAP_FAILED) {
> >           goto error_close_unlink;
> >       }
> > -    addr = mmap (addr_orig, bytes, PROT_READ | PROT_WRITE,
> > -        MAP_FIXED | MAP_SHARED, fd, 0);
> > +    addr = mmap (addr_orig, bytes, PROT_READ | PROT_WRITE, flags, fd,
> > 0);
> >       if (addr != addr_orig) {
> >           munmap(addr_orig, bytes);
> > diff --git a/lib/cpg.c b/lib/cpg.c
> > index b96df4e..7cc3ce9 100644
> > --- a/lib/cpg.c
> > +++ b/lib/cpg.c
> > @@ -737,6 +737,7 @@ memory_map (char *path, const char *file, void
> > **buf, size_t bytes)
> >       int32_t i;
> >       size_t written;
> >       size_t page_size;
> > +    int flags = MAP_ANONYMOUS;
> >       snprintf (path, PATH_MAX, "/dev/shm/%s", file);
> > @@ -772,8 +773,13 @@ retry_write:
> >       }
> >       free (buffer);
> > -    addr_orig = mmap (NULL, bytes, PROT_NONE,
> > -        MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> > +#ifdef FORCE_SHM_ALIGN
> > +    flags |= MAP_SHARED;
> > +#else
> > +    flags |= MAP_PRIVATE;
> > +#endif /* FORCE_SHM_ALIGN */
> > +
> > +    addr_orig = mmap (NULL, bytes, PROT_NONE, flags, -1, 0);
> >       if (addr_orig == MAP_FAILED) {
> >           goto error_close_unlink;

--
Best Reagrds,
Roy | RongQing Li
_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss