Re: [PATCH] Fix shared memory mapping

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 05/20/2013 09:59 PM, Jan Friesse wrote:
In addition to Fabio's comment. I believe whole original code is just
nonsense. I've merged these two mappings into one and it looks like
functionality is kept (patch Remove unnecessary mmap in cpg).

Roy,
do you think you can try that patch and tell me, if it solves also
problem you are hitting? (it should)


Yes, I like, Could you send your patch to me?

-Roy

Regards,
   Honza

Fabio M. Di Nitto napsal(a):
On 5/19/2013 3:00 AM, 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.

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

Hi,

this part of the patch is not ok. We spent a decent amount of time
dropping all hard coded architecture bits from configure.ac because they
are not portable.

You will need to find a more generic test to verify if shmlba is
necessary on a certain architecture/OS. Remember that we do support more
than just Linux for upstream.

Cheers
Fabio

_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss

_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss


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




[Index of Archives]     [Linux Clusters]     [Corosync Project]     [Linux USB Devel]     [Linux Audio Users]     [Photo]     [Yosemite News]    [Yosemite Photos]    [Linux Kernel]     [Linux SCSI]     [X.Org]

  Powered by Linux