1

I am trying to change the value of a position of an array inside a struct that is allocated in shared memory I have this struct:

typedef struct Paths {
     int *path;
     int totalDistance;
     int quantity;
}Path;

And i have this algorithm:

void runAlgorithm(int** distances,int nCities, int executionTime, int nProcesses){
int size = nCities+2 * sizeof(int);
int protection = PROT_READ | PROT_WRITE;
int visibility = MAP_ANONYMOUS | MAP_SHARED;
PtPath shmem = (PtPath)mmap(NULL, size, protection, visibility, 0, 0);
*shmem = createPath(nCities);
randomPath(shmem);
setPathTotalDistance(shmem, distances);
sem_unlink("job_ready");
sem_unlink("job_done");
sem_t *job_ready = sem_open("job_ready", O_CREAT, 0644, 0);
sem_t *job_done = sem_open("job_done", O_CREAT, 0644, 0);

int *pids = (int*)calloc(nProcesses, sizeof(int));
Path path, shortestPath;

//Workers Processes
for(int i = 0 ; i < nProcesses ; i++){
    pids[i] = fork();
    if(pids[i] == 0){
        shortestPath = createPath(nCities);
        randomPath(&shortestPath); //inicializa caminho aleatorio
        setPathTotalDistance(&shortestPath, distances); //problema aqui 
        while(1){
            sem_wait(job_ready);                
            mutation(&shortestPath, distances);
            if(shortestPath.totalDistance < shmem->totalDistance){
                printPath(shmem);
                printPath(&shortestPath);
                copyPath(&shortestPath, shmem);
            }
            sem_post(job_done);
        }
        exit(0);
    }
}

//Parent Process
int elapsed, terminate = 1;
time_t start = time(NULL), end;
printf("Elapsed time: \n");
while(terminate){
    end = time(NULL);
    elapsed = difftime(end,start);
    printElapsedTime(elapsed);
    if(elapsed >= executionTime)
        terminate = 0;
    else{
        sem_post(job_ready);
        sem_wait(job_done);
    }
}
printPath(shmem);
sem_close(job_ready);
sem_close(job_done);

// Kill worker processes
for (int i=0; i<nProcesses; i++) {
    printf("Killing %d\n", pids[i]);
    kill(pids[i], SIGKILL);
}
}

This is the code of createPath:

Path createPath(int quantity){
Path newPath;

if(quantity < 0)
    quantity = 0;
newPath.path = (int*)calloc(quantity, sizeof(int));
newPath.quantity = quantity;
newPath.totalDistance = 0;

return newPath;
}

And this is the copyPath code:

void copyPath(PtPath from, PtPath to){
for(int i = 0 ; i < from->quantity ; i++){
    //printf("posI: %d\n",to->path[i]);
    to->path[i] = from->path[i];
}
to->totalDistance = from->totalDistance;
to->quantity = from->quantity;
}

So my problem is i'm trying to copy the shortestPath to the shared memory and it copies nothing, but it does copy the totalDistance What am i doing wrong and where?

Hugo Modesto
  • 63
  • 1
  • 8
  • What is the variable ```size``` in the second code block? Is it the ```sizeof(Paths)```? – JiaHao Xu Dec 05 '18 at 21:37
  • The `path` field of your `struct Paths` is a **pointer**. What memory do you think `shmem->path` points to? – Andrew Henle Dec 05 '18 at 21:38
  • If you want to use shared memory, then does this mean that you have another process(thread) tries to read this? – JiaHao Xu Dec 05 '18 at 21:40
  • `shmem->path` is a pointer to an array of integer, not an array. I guess `size` is big enough for the struct and the array, so I guess after the allocation you should do `shmem->path = (int *)(shmem + 1)` which will put the array after the struct in the shared memory – Stefan Becker Dec 05 '18 at 21:41
  • @Stefan Becker If the shared memory is used in another process(thread), it won't be able to access ```path```. The OP has to clarify how he will use ```shmem```. – JiaHao Xu Dec 05 '18 at 21:43
  • @JiaHaoXu that problem can simply be solved by removing `path` from the struct and use `int *path = (int *)(shmem + 1)` everywhere where `shmem` is accessed. – Stefan Becker Dec 05 '18 at 22:01
  • Of course another solution is to re-order the struct and put the array at the end, i.e. `{ int totalDistance; int size; int path[0]; }` and then allocate `sizeof(struct ...) + (size - 1)*sizeof(int)` of shared memory. Then the OP code should work unmodified. – Stefan Becker Dec 05 '18 at 22:05
  • @Stefan Becker If the shared memory is mapped tonother address, this method will immediately fail. Either the other process(thread) use ```mmap``` to load it (though it can't be done here) or its child process use ```mremap``` to change the size. – JiaHao Xu Dec 05 '18 at 22:05
  • It is better to rewrite the structure to adjust to the requirements of shared memory than placing assumptions on how it is used. – JiaHao Xu Dec 05 '18 at 22:09
  • @JiaHaoXu only if you forget to recalculate `path` after you re-mapped the shared memory. – Stefan Becker Dec 05 '18 at 22:11
  • @JiaHaoXu Size is equals to size of array(which is defined as number of cities)+2(because of the other 2 attribute i have on that struct) * sizeof(int) – Hugo Modesto Dec 05 '18 at 22:24
  • No, you needed another ```sizeof(int*)``` for storing the pointer to the array. – JiaHao Xu Dec 05 '18 at 22:25
  • @AndrewHenle i think it points like a normal array, i mean, when i call createPath() it allocates the memory that i need to use the array normally, so after that ill be able to use like the "newPath" ex.: shmem->path[0] – Hugo Modesto Dec 05 '18 at 22:25
  • It is better to let ```size``` = ```sizeof(Paths) + /* sizeof array path*/```. – JiaHao Xu Dec 05 '18 at 22:26
  • @JiaHaoXu ahhhh ok, thanks!! – Hugo Modesto Dec 05 '18 at 22:26
  • And you add tag ```fork``` to this question, does this mean that the shared memory will be used by its child (from the way you setup shared memory, I guess you want to use it this way)? – JiaHao Xu Dec 05 '18 at 22:27
  • @JiaHaoXu about the second comment you did, yes, i have 1 parent process, and x child processes, and i need to "share" a shortestPath with the parent – Hugo Modesto Dec 05 '18 at 22:27
  • Better to edit your question and clarify all of them. – JiaHao Xu Dec 05 '18 at 22:27
  • @JiaHaoXu i didnt post the whole code because i though it was too big to post it here – Hugo Modesto Dec 05 '18 at 22:28
  • Just clarify what is ```size``` and how are you gonna to use the shared memory. – JiaHao Xu Dec 05 '18 at 22:29
  • BTW, the sizeof the array path is not ```nCities```, it is ```nCities * sizeof(int)```. – JiaHao Xu Dec 05 '18 at 23:00
  • Another advice: You may want to use thread instead of process so that you won't have to use shared memory any more and you don't have to worry about recording them and terminating threads one by one, simply ```exit_group(/* status_code */)``` will do the job. And it is more lightweight. – JiaHao Xu Dec 05 '18 at 23:25
  • @JiaHaoXu i know that Jia, but this is a college project and in this version of the project i am not allowed, yet, to use threads – Hugo Modesto Dec 05 '18 at 23:37

1 Answers1

1

Since you want to use shared memory, then I recommand you to rewrite the structure since the address of the shared memory can be changed when mmaping (not possible here) or mremap.

IMHO, it is always better to use offset inside shared memory than using pointer and assuming the address won't be changed, even if you can sure the address won't change now.

So:

struct Paths
{
    int totalDistance;
    int quantity;
    int path[0];
};

Since path is stored inlined with Paths continuously, there is no need for int *path.

When you allocate memory using mmap, the size should be sizeof(Paths) plus the size you need for storing path.

size_t size = sizeof(Paths) + /* Additional size needed for storing path */;
int protection = PROT_READ | PROT_WRITE;
int visibility = MAP_ANONYMOUS | MAP_SHARED; 
PtPath shmem = (PtPath) mmap(NULL, size, protection, visibility, -1, 0);

When you accessing path from Paths, the compiler will use offset from the address of Paths to access it, so as long as you hold a valid pointer to a shared memory with valid Paths, you can access it no matter it is mmaped ormremaped.

Coping the data from newPath will be just:

Path newPath = createPath();
for(int i = 0 ; i < shmem->quantity ; i++)
    shmem->path[i] = newPath.path[i];

This method has a slight advantage over the method used in the question: it does not need any additional pointer, which might be useful if you allocate a lot of Path.

Now, it is better to use void initializePath(Path*, int) instead of Path createPath(int):

void initializePath(Path *newPath, int quantity)
{
    if (quantity < 0)
        quantity = 0;
    newPath->quantity = quantity;
    newPath->totalDistance = 0;

    randomPath(shmem);       
    setPathTotalDistance(shmem, distances);
}

And you can have another function Path* allocatePath(int) to allocate Path:

Path* allocateSharedPath(int quantity)
{
    size_t size = sizeof(Paths) + quantity * sizeof(int);
    int protection = PROT_READ | PROT_WRITE;
    int visibility = MAP_ANONYMOUS | MAP_SHARED; 

    Path *shmem = (Path*) mmap(NULL, size, protection, visibility, -1, 0);

    if (shmem == NULL)
        /* Handle failure */;

    return shmem;
}

Path* allocatePath(int quantity)
{
    size_t size = sizeof(Paths) + quantity * sizeof(int);

    Path *mem = (Path*) calloc(size, 1);

    if (mem == NULL)
        /* Handle failure */;

    return mem;
}

BTW, if you want to store something else between quantity and path or you want to store path before Paths you can do this:

struct Paths
{
    int totalDistance;
    int quantity;
    offset_t offset_to_path;
};

The size required to allocate is sizeof(Paths) + /* Additional size needed for storing path */ + /* sizeof whatever else you need to store */. When initializing, initialize offset_to_path to pointer_to_path - pointer_to_Paths so that you can access it by adding offset_to_path to the pointer to Paths.

JiaHao Xu
  • 2,452
  • 16
  • 31