Re: [PATCH] Remove unnecessary mmap in memory_map ipc

[Rongqing Li <rongqing.li@xxxxxxxxxxxxx>]

This patch fixed the bug on MIPS, and works well.

But I have a question, should we do the same change in
circular_memory_map() ? which are using mmap() three times.


-RongQing


On 05/23/2013 03:25 AM, Fabio M. Di Nitto wrote:
> Looks good.
>
> Fabio
>
> On 05/22/2013 05:53 PM, Jan Friesse wrote:
> > This is similar patch as master e684e4ca6fed709c14d79d8d81f254aa48e1c65a
> > but for whole IPC.
> >
> > Signed-off-by: Jan Friesse <jfriesse@xxxxxxxxxx>
> > ---
> >   exec/coroipcs.c |   17 ++++-------------
> >   lib/coroipcc.c  |   17 +++++------------
> >   2 files changed, 9 insertions(+), 25 deletions(-)
> >
> > diff --git a/exec/coroipcs.c b/exec/coroipcs.c
> > index 7645499..39f7ff7 100644
> > --- a/exec/coroipcs.c
> > +++ b/exec/coroipcs.c
> > @@ -222,7 +222,6 @@ memory_map (
> >   	void **buf)
> >   {
> >   	int32_t fd;
> > -	void *addr_orig;
> >   	void *addr;
> >   	int32_t res;
> >
> > @@ -239,18 +238,10 @@ 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);
> > +	if (addr == MAP_FAILED) {
> >   		goto error_close_unlink;
> >   	}
> >   #if (defined COROSYNC_BSD && defined MADV_NOSYNC)
> > @@ -261,7 +252,7 @@ memory_map (
> >   	if (res) {
> >   		return (-1);
> >   	}
> > -	*buf = addr_orig;
> > +	*buf = addr;
> >   	return (0);
> >
> >   error_close_unlink:
> > diff --git a/lib/coroipcc.c b/lib/coroipcc.c
> > index 140fa18..b3a074f 100644
> > --- a/lib/coroipcc.c
> > +++ b/lib/coroipcc.c
> > @@ -405,7 +405,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;
> > @@ -451,28 +450,22 @@ retry_write:
> >   	}
> >   	free (buffer);
> >
> > -	addr_orig = mmap (NULL, bytes, PROT_NONE,
> > -		MAP_ANONYMOUS | MAP_PRIVATE, -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 (NULL, bytes, PROT_READ | PROT_WRITE,
> > +		MAP_SHARED, fd, 0);
> >
> > -	if (addr != addr_orig) {
> > +	if (addr == MAP_FAILED) {
> >   		goto error_close_unlink;
> >   	}
> >   #if (defined COROSYNC_BSD && defined 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;
>
> _______________________________________________
> 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