Rongqing Li napsal(a):
>
>
> On 05/28/2013 03:27 AM, Steven Dake wrote:
>> On 05/27/2013 08:39 AM, Jan Friesse wrote:
>>> Rongqing Li napsal(a):
>>>> This patch fixed the bug on MIPS, and works well.
>>>>
>>> Rongqing,
>>> I'm not sure WHY circular_memory_map (or 3 mmaps used there) works, but
>>> as long as it works, I would probably keep it as it is.
>>>
>> Honza,
>>
>> 1. The first mmap maps an address (like malloc) that is 2x the size of
>> the memory buffer.
>>
>> 2. The second maps a memory buffer into the first block of memory done
>> in step 1.
>>
>> 3. The third mmap memory maps that same buffer into the second block of
>> memory in step 1.
>>
>> This results in a buffer that looks like this
>>
>> |BUFFER|BUFFER|
>>
>> If a read/write occurs at buffer 1 R, it will continue on into buffer 2
>> BUFF, removing the need for special logic to handle end of buffer
>> read/write conditions.
>
> Thanks for your explanation.
>
> But I am not clearing what is the end of buffer read/write condition,
It's ring buffer (as Steve explained), so it would need to check whether:
- current pos + length of new data <= rb size
- if so, write
- if not, write only part of new_data, set current pos to 0 and write
rest of data
What is harder then what current code does.
Honza
> why is it needed to be handle specially.
>
>
> -Roy
>
>>
>> This dramatically improved reliability and performance, I suppose at the
>> cost of portability to some non-cache-coherent hardware.
>>
>> It may be possible to add a portability mode that doesn't use the mmap
>> (and instead calculates and copies only blocks it needs) but I'm not
>> quite sure how that would operate and seems to be at risk of regression.
>>
>> Regards
>> -steve
>>
>>>> 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
>>>>>
>>> _______________________________________________
>>> discuss mailing list
>>> discuss@xxxxxxxxxxxx
>>> http://lists.corosync.org/mailman/listinfo/discuss
>>
>>
>>
>
_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss
