Re: [PATCH] Remove unnecessary mmap in cpg

[Rongqing Li <rongqing.li@xxxxxxxxxxxxx>]

On 05/22/2013 11:58 PM, Jan Friesse wrote:
> Rongqing,
> I've tried to create that patch (on ML as "Remove unnecessary mmap in
> memory_map ipc"), so can you please test it and share your results?
>
> Also there is circular_memory_map function and there we sadly need to
> really map one file to two regions of continuous memory. There seems to
> be really only one way how to achieve and this is to 1. allocate big
> chunk (2 * size) and then use mmap with FIXED flag to mmap file two
> times, what may be not working on MIPS (any ideas how to achieve this
> functionality is welcomed).

Sorry, I just see this mail, and know the reply on circular_memory_map.

Could we replace MAP_PRIVATE with MAP_SHARE to fix it?

Thanks.


-RongQing



> Regards,
>    Honza
>
> Rongqing Li napsal(a):
> > Could I backport this patch to corosync 1.4.3?
> >
> > I backport it to 1.4.3, and then run the openais self test case,
> > and many test case failed.
> >
> >
> > -Roy
> >
> > On 05/21/2013 09:01 AM, Rongqing Li wrote:
> > > Ok, I will test it soon.
> > > Thanks
> > >
> > > -Roy
> > >
> > >
> > > On 05/20/2013 09:57 PM, Jan Friesse wrote:
> > > > Code for zero-copy in cpg does following mmaps:
> > > > - Mmap anonymous, private memory to some address (-> malloc)
> > > > - Mmap shared memory of fd to address returned by first mmap
> > > >     (effectively shadows first mapping)
> > > >
> > > > This is not necessary and only one mapping is needed.
> > > >
> > > > Signed-off-by: Jan Friesse <jfriesse@xxxxxxxxxx>
> > > > ---
> > > >    exec/cpg.c |   18 +++---------------
> > > >    lib/cpg.c  |   19 ++++---------------
> > > >    2 files changed, 7 insertions(+), 30 deletions(-)
> > > >
> > > > diff --git a/exec/cpg.c b/exec/cpg.c
> > > > index 695f311..bfc3813 100644
> > > > --- a/exec/cpg.c
> > > > +++ b/exec/cpg.c
> > > > @@ -1603,7 +1603,6 @@ memory_map (
> > > >        void **buf)
> > > >    {
> > > >        int32_t fd;
> > > > -    void *addr_orig;
> > > >        void *addr;
> > > >        int32_t res;
> > > >
> > > > @@ -1620,20 +1619,9 @@ memory_map (
> > > >            goto error_close_unlink;
> > > >        }
> > > >
> > > > -    addr_orig = mmap (NULL, bytes, PROT_NONE,
> > > > -        MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> > > > +    addr = mmap (NULL, bytes, PROT_READ | PROT_WRITE,
> > > > +        MAP_SHARED, fd, 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);
> > > > -
> > > > -    if (addr != addr_orig) {
> > > > -        munmap(addr_orig, bytes);
> > > > -        goto error_close_unlink;
> > > > -    }
> > > >    #ifdef MADV_NOSYNC
> > > >        madvise(addr, bytes, MADV_NOSYNC);
> > > >    #endif
> > > > @@ -1642,7 +1630,7 @@ memory_map (
> > > >        if (res) {
> > > >            return (-1);
> > > >        }
> > > > -    *buf = addr_orig;
> > > > +    *buf = addr;
> > > >        return (0);
> > > >
> > > >    error_close_unlink:
> > > > diff --git a/lib/cpg.c b/lib/cpg.c
> > > > index b96df4e..f0ac593 100644
> > > > --- a/lib/cpg.c
> > > > +++ b/lib/cpg.c
> > > > @@ -730,7 +730,6 @@ static int
> > > >    memory_map (char *path, const char *file, void **buf, size_t bytes)
> > > >    {
> > > >        int32_t fd;
> > > > -    void *addr_orig;
> > > >        void *addr;
> > > >        int32_t res;
> > > >        char *buffer;
> > > > @@ -772,28 +771,18 @@ retry_write:
> > > >        }
> > > >        free (buffer);
> > > >
> > > > -    addr_orig = mmap (NULL, bytes, PROT_NONE,
> > > > -        MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> > > > +    addr = mmap (NULL, bytes, PROT_READ | PROT_WRITE,
> > > > +        MAP_SHARED, fd, 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);
> > > > -
> > > > -    if (addr != addr_orig) {
> > > > -        goto error_close_unlink;
> > > > -    }
> > > >    #ifdef MADV_NOSYNC
> > > > -    madvise(addr_orig, bytes, MADV_NOSYNC);
> > > > +    madvise(addr, bytes, MADV_NOSYNC);
> > > >    #endif
> > > >
> > > >        res = close (fd);
> > > >        if (res) {
> > > >            return (-1);
> > > >        }
> > > > -    *buf = addr_orig;
> > > > +    *buf = addr;
> > > >
> > > >        return 0;

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