Skip to content

Commit ee80bab

Browse files
devrealdevreal
committed
Remove unused opal_shmem_seg_hdr_t to retain alignment
Signed-off-by: Joseph Schuchart <[email protected]>
1 parent b50d568 commit ee80bab

File tree

4 files changed

+42
-98
lines changed

4 files changed

+42
-98
lines changed

‎opal/mca/shmem/mmap/shmem_mmap_module.c

Lines changed: 16 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ shmem_ds_reset(opal_shmem_ds_t *ds_buf)
133133
ds_buf->seg_id = OPAL_SHMEM_DS_ID_INVALID;
134134
ds_buf->seg_size = 0;
135135
memset(ds_buf->seg_name, '\0', OPAL_PATH_MAX);
136-
ds_buf->seg_base_addr = (unsigned char *)MAP_FAILED;
136+
ds_buf->seg_base_addr = MAP_FAILED;
137137
}
138138

139139
/* ////////////////////////////////////////////////////////////////////////// */
@@ -303,12 +303,7 @@ segment_create(opal_shmem_ds_t *ds_buf,
303303
pid_t my_pid = getpid();
304304
bool space_available = false;
305305
uint64_t amount_space_avail = 0;
306-
307-
/* the real size of the shared memory segment. this includes enough space
308-
* to store our segment header.
309-
*/
310-
size_t real_size = size + sizeof(opal_shmem_seg_hdr_t);
311-
opal_shmem_seg_hdr_t *seg_hdrp = MAP_FAILED;
306+
void *segment = MAP_FAILED;
312307

313308
/* init the contents of opal_shmem_ds_t */
314309
shmem_ds_reset(ds_buf);
@@ -372,8 +367,7 @@ segment_create(opal_shmem_ds_t *ds_buf,
372367
real_file_name);
373368
}
374369
/* let's make sure we have enough space for the backing file */
375-
if (OPAL_SUCCESS != (rc = enough_space(real_file_name,
376-
real_size,
370+
if (OPAL_SUCCESS != (rc = enough_space(real_file_name, size,
377371
&amount_space_avail,
378372
&space_available))) {
379373
opal_output(0, "shmem: mmap: an error occurred while determining "
@@ -386,7 +380,7 @@ segment_create(opal_shmem_ds_t *ds_buf,
386380
gethostname(hn, sizeof(hn));
387381
rc = OPAL_ERR_OUT_OF_RESOURCE;
388382
opal_show_help("help-opal-shmem-mmap.txt", "target full", 1,
389-
real_file_name, hn, (unsigned long)real_size,
383+
real_file_name, hn, (unsigned long)size,
390384
(unsigned long long)amount_space_avail);
391385
goto out;
392386
}
@@ -400,8 +394,8 @@ segment_create(opal_shmem_ds_t *ds_buf,
400394
rc = OPAL_ERROR;
401395
goto out;
402396
}
403-
/* size backing file - note the use of real_size here */
404-
if (0 != ftruncate(ds_buf->seg_id, real_size)) {
397+
/* size backing file */
398+
if (0 != ftruncate(ds_buf->seg_id, size)) {
405399
int err = errno;
406400
char hn[OPAL_MAXHOSTNAMELEN];
407401
gethostname(hn, sizeof(hn));
@@ -410,8 +404,7 @@ segment_create(opal_shmem_ds_t *ds_buf,
410404
rc = OPAL_ERROR;
411405
goto out;
412406
}
413-
if (MAP_FAILED == (seg_hdrp = (opal_shmem_seg_hdr_t *)
414-
mmap(NULL, real_size,
407+
if (MAP_FAILED == (segment = mmap(NULL, size,
415408
PROT_READ | PROT_WRITE, MAP_SHARED,
416409
ds_buf->seg_id, 0))) {
417410
int err = errno;
@@ -424,20 +417,11 @@ segment_create(opal_shmem_ds_t *ds_buf,
424417
}
425418
/* all is well */
426419
else {
427-
/* -- initialize the shared memory segment -- */
428-
opal_atomic_rmb();
429-
430-
/* init segment lock */
431-
opal_atomic_lock_init(&seg_hdrp->lock, OPAL_ATOMIC_LOCK_UNLOCKED);
432-
/* i was the creator of this segment, so note that fact */
433-
seg_hdrp->cpid = my_pid;
434-
435-
opal_atomic_wmb();
436420

437421
/* -- initialize the contents of opal_shmem_ds_t -- */
438422
ds_buf->seg_cpid = my_pid;
439-
ds_buf->seg_size = real_size;
440-
ds_buf->seg_base_addr = (unsigned char *)seg_hdrp;
423+
ds_buf->seg_size = size;
424+
ds_buf->seg_base_addr = segment;
441425
(void)opal_string_copy(ds_buf->seg_name, real_file_name, OPAL_PATH_MAX);
442426

443427
/* set "valid" bit because setment creation was successful */
@@ -471,8 +455,8 @@ segment_create(opal_shmem_ds_t *ds_buf,
471455
}
472456
/* an error occured, so invalidate the shmem object and munmap if needed */
473457
if (OPAL_SUCCESS != rc) {
474-
if (MAP_FAILED != seg_hdrp) {
475-
munmap((void *)seg_hdrp, real_size);
458+
if (MAP_FAILED != segment) {
459+
munmap(segment, size);
476460
}
477461
shmem_ds_reset(ds_buf);
478462
}
@@ -501,10 +485,9 @@ segment_attach(opal_shmem_ds_t *ds_buf)
501485
"open(2)", "", strerror(err), err);
502486
return NULL;
503487
}
504-
if (MAP_FAILED == (ds_buf->seg_base_addr = (unsigned char *)
505-
mmap(NULL, ds_buf->seg_size,
506-
PROT_READ | PROT_WRITE, MAP_SHARED,
507-
ds_buf->seg_id, 0))) {
488+
if (MAP_FAILED == (ds_buf->seg_base_addr = mmap(NULL, ds_buf->seg_size,
489+
PROT_READ | PROT_WRITE, MAP_SHARED,
490+
ds_buf->seg_id, 0))) {
508491
int err = errno;
509492
char hn[OPAL_MAXHOSTNAMELEN];
510493
gethostname(hn, sizeof(hn));
@@ -542,7 +525,7 @@ segment_attach(opal_shmem_ds_t *ds_buf)
542525
);
543526

544527
/* update returned base pointer with an offset that hides our stuff */
545-
return (ds_buf->seg_base_addr + sizeof(opal_shmem_seg_hdr_t));
528+
return ds_buf->seg_base_addr;
546529
}
547530

548531
/* ////////////////////////////////////////////////////////////////////////// */
@@ -560,7 +543,7 @@ segment_detach(opal_shmem_ds_t *ds_buf)
560543
ds_buf->seg_id, (unsigned long)ds_buf->seg_size, ds_buf->seg_name)
561544
);
562545

563-
if (0 != munmap((void *)ds_buf->seg_base_addr, ds_buf->seg_size)) {
546+
if (0 != munmap(ds_buf->seg_base_addr, ds_buf->seg_size)) {
564547
int err = errno;
565548
char hn[OPAL_MAXHOSTNAMELEN];
566549
gethostname(hn, sizeof(hn));

‎opal/mca/shmem/posix/shmem_posix_module.c

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ shmem_ds_reset(opal_shmem_ds_t *ds_buf)
123123
ds_buf->seg_id = OPAL_SHMEM_DS_ID_INVALID;
124124
ds_buf->seg_size = 0;
125125
memset(ds_buf->seg_name, '\0', OPAL_PATH_MAX);
126-
ds_buf->seg_base_addr = (unsigned char *)MAP_FAILED;
126+
ds_buf->seg_base_addr = MAP_FAILED;
127127
}
128128

129129
/* ////////////////////////////////////////////////////////////////////////// */
@@ -174,11 +174,7 @@ segment_create(opal_shmem_ds_t *ds_buf,
174174
{
175175
int rc = OPAL_SUCCESS;
176176
pid_t my_pid = getpid();
177-
/* the real size of the shared memory segment. this includes enough space
178-
* to store our segment header.
179-
*/
180-
size_t real_size = size + sizeof(opal_shmem_seg_hdr_t);
181-
opal_shmem_seg_hdr_t *seg_hdrp = MAP_FAILED;
177+
void *segment = MAP_FAILED;
182178

183179
/* init the contents of opal_shmem_ds_t */
184180
shmem_ds_reset(ds_buf);
@@ -200,8 +196,8 @@ segment_create(opal_shmem_ds_t *ds_buf,
200196
rc = OPAL_ERROR;
201197
goto out;
202198
}
203-
/* size backing file - note the use of real_size here */
204-
else if (0 != ftruncate(ds_buf->seg_id, real_size)) {
199+
/* size backing file */
200+
else if (0 != ftruncate(ds_buf->seg_id, size)) {
205201
int err = errno;
206202
char hn[OPAL_MAXHOSTNAMELEN];
207203
gethostname(hn, sizeof(hn));
@@ -210,9 +206,9 @@ segment_create(opal_shmem_ds_t *ds_buf,
210206
rc = OPAL_ERROR;
211207
goto out;
212208
}
213-
else if (MAP_FAILED == (seg_hdrp = (opal_shmem_seg_hdr_t*)mmap(NULL, real_size,
214-
PROT_READ | PROT_WRITE, MAP_SHARED,
215-
ds_buf->seg_id, 0))) {
209+
else if (MAP_FAILED == (segment = mmap(NULL, size,
210+
PROT_READ | PROT_WRITE, MAP_SHARED,
211+
ds_buf->seg_id, 0))) {
216212
int err = errno;
217213
char hn[OPAL_MAXHOSTNAMELEN];
218214
gethostname(hn, sizeof(hn));
@@ -223,20 +219,11 @@ segment_create(opal_shmem_ds_t *ds_buf,
223219
}
224220
/* all is well */
225221
else {
226-
/* -- initialize the shared memory segment -- */
227-
opal_atomic_rmb();
228-
229-
/* init segment lock */
230-
opal_atomic_lock_init(&seg_hdrp->lock, OPAL_ATOMIC_LOCK_UNLOCKED);
231-
/* i was the creator of this segment, so note that fact */
232-
seg_hdrp->cpid = my_pid;
233-
234-
opal_atomic_wmb();
235222

236223
/* -- initialize the contents of opal_shmem_ds_t -- */
237224
ds_buf->seg_cpid = my_pid;
238-
ds_buf->seg_size = real_size;
239-
ds_buf->seg_base_addr = (unsigned char *)seg_hdrp;
225+
ds_buf->seg_size = size;
226+
ds_buf->seg_base_addr = segment;
240227

241228
/* notice that we are not setting ds_buf->name here. at this point,
242229
* posix_shm_open was successful, so the contents of ds_buf->name are
@@ -283,8 +270,8 @@ segment_create(opal_shmem_ds_t *ds_buf,
283270
if (-1 != ds_buf->seg_id) {
284271
shm_unlink(ds_buf->seg_name);
285272
}
286-
if (MAP_FAILED != seg_hdrp) {
287-
munmap((void*)seg_hdrp, real_size);
273+
if (MAP_FAILED != segment) {
274+
munmap((void*)segment, size);
288275
}
289276
/* always invalidate in this error path */
290277
shmem_ds_reset(ds_buf);
@@ -310,10 +297,9 @@ segment_attach(opal_shmem_ds_t *ds_buf)
310297
"open(2)", "", strerror(err), err);
311298
return NULL;
312299
}
313-
else if (MAP_FAILED == (ds_buf->seg_base_addr =
314-
(unsigned char*)mmap(NULL, ds_buf->seg_size,
315-
PROT_READ | PROT_WRITE, MAP_SHARED,
316-
ds_buf->seg_id, 0))) {
300+
else if (MAP_FAILED == (ds_buf->seg_base_addr = mmap(NULL, ds_buf->seg_size,
301+
PROT_READ | PROT_WRITE, MAP_SHARED,
302+
ds_buf->seg_id, 0))) {
317303
int err = errno;
318304
char hn[OPAL_MAXHOSTNAMELEN];
319305
gethostname(hn, sizeof(hn));
@@ -353,7 +339,7 @@ segment_attach(opal_shmem_ds_t *ds_buf)
353339
);
354340

355341
/* update returned base pointer with an offset that hides our stuff */
356-
return (ds_buf->seg_base_addr + sizeof(opal_shmem_seg_hdr_t));
342+
return ds_buf->seg_base_addr;
357343
}
358344

359345
/* ////////////////////////////////////////////////////////////////////////// */
@@ -371,7 +357,7 @@ segment_detach(opal_shmem_ds_t *ds_buf)
371357
ds_buf->seg_id, (unsigned long)ds_buf->seg_size, ds_buf->seg_name)
372358
);
373359

374-
if (0 != munmap((void*)ds_buf->seg_base_addr, ds_buf->seg_size)) {
360+
if (0 != munmap(ds_buf->seg_base_addr, ds_buf->seg_size)) {
375361
int err = errno;
376362
char hn[OPAL_MAXHOSTNAMELEN];
377363
gethostname(hn, sizeof(hn));

‎opal/mca/shmem/shmem_types.h

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
#define OPAL_SHMEM_TYPES_H
3333

3434
#include "opal_config.h"
35-
#include "opal/align.h"
3635

3736
#include <stddef.h>
3837
#include <string.h>
@@ -98,15 +97,6 @@ do { \
9897

9998
typedef uint8_t opal_shmem_ds_flag_t;
10099

101-
/* shared memory segment header */
102-
struct opal_shmem_seg_hdr_t {
103-
/* segment lock */
104-
opal_atomic_lock_t lock;
105-
/* pid of the segment creator */
106-
pid_t cpid;
107-
} __opal_attribute_aligned__(OPAL_ALIGN_MIN);
108-
typedef struct opal_shmem_seg_hdr_t opal_shmem_seg_hdr_t;
109-
110100
struct opal_shmem_ds_t {
111101
/* pid of the shared memory segment creator */
112102
pid_t seg_cpid;
@@ -117,7 +107,7 @@ struct opal_shmem_ds_t {
117107
/* size of shared memory segment */
118108
size_t seg_size;
119109
/* base address of shared memory segment */
120-
unsigned char *seg_base_addr;
110+
void *seg_base_addr;
121111
/* path to backing store -- last element so we can easily calculate the
122112
* "real" size of opal_shmem_ds_t. that is, the amount of the struct that
123113
* is actually being used. for example: if seg_name is something like:

‎opal/mca/shmem/sysv/shmem_sysv_module.c

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -176,11 +176,7 @@ segment_create(opal_shmem_ds_t *ds_buf,
176176
{
177177
int rc = OPAL_SUCCESS;
178178
pid_t my_pid = getpid();
179-
/* the real size of the shared memory segment. this includes enough space
180-
* to store our segment header.
181-
*/
182-
size_t real_size = size + sizeof(opal_shmem_seg_hdr_t);
183-
opal_shmem_seg_hdr_t *seg_hdrp = MAP_FAILED;
179+
void *segment = MAP_FAILED;
184180

185181
/* init the contents of opal_shmem_ds_t */
186182
shmem_ds_reset(ds_buf);
@@ -189,10 +185,8 @@ segment_create(opal_shmem_ds_t *ds_buf,
189185
* being located on a network file system... so no check is needed here.
190186
*/
191187

192-
/* create a new shared memory segment and save the shmid. note the use of
193-
* real_size here
194-
*/
195-
if (-1 == (ds_buf->seg_id = shmget(IPC_PRIVATE, real_size,
188+
/* create a new shared memory segment and save the shmid. */
189+
if (-1 == (ds_buf->seg_id = shmget(IPC_PRIVATE, size,
196190
IPC_CREAT | IPC_EXCL | S_IRWXU))) {
197191
int err = errno;
198192
char hn[OPAL_MAXHOSTNAMELEN];
@@ -203,7 +197,7 @@ segment_create(opal_shmem_ds_t *ds_buf,
203197
goto out;
204198
}
205199
/* attach to the sement */
206-
else if ((void *)-1 == (seg_hdrp = shmat(ds_buf->seg_id, NULL, 0))) {
200+
else if ((void *)-1 == (segment = shmat(ds_buf->seg_id, NULL, 0))) {
207201
int err = errno;
208202
char hn[OPAL_MAXHOSTNAMELEN];
209203
gethostname(hn, sizeof(hn));
@@ -228,20 +222,11 @@ segment_create(opal_shmem_ds_t *ds_buf,
228222
}
229223
/* all is well */
230224
else {
231-
/* -- initialize the shared memory segment -- */
232-
opal_atomic_rmb();
233-
234-
/* init segment lock */
235-
opal_atomic_lock_init(&seg_hdrp->lock, OPAL_ATOMIC_LOCK_UNLOCKED);
236-
/* i was the creator of this segment, so note that fact */
237-
seg_hdrp->cpid = my_pid;
238-
239-
opal_atomic_wmb();
240225

241226
/* -- initialize the contents of opal_shmem_ds_t -- */
242227
ds_buf->seg_cpid = my_pid;
243-
ds_buf->seg_size = real_size;
244-
ds_buf->seg_base_addr = (unsigned char *)seg_hdrp;
228+
ds_buf->seg_size = size;
229+
ds_buf->seg_base_addr = (unsigned char *)segment;
245230

246231
/* notice that we are not setting ds_buf->name here. sysv doesn't use
247232
* it, so don't worry about it - shmem_ds_reset took care of
@@ -267,8 +252,8 @@ segment_create(opal_shmem_ds_t *ds_buf,
267252
*/
268253
if (OPAL_SUCCESS != rc) {
269254
/* best effort to delete the segment. */
270-
if ((void *)-1 != seg_hdrp) {
271-
shmdt((char*)seg_hdrp);
255+
if ((void *)-1 != segment) {
256+
shmdt((char*)segment);
272257
}
273258
shmctl(ds_buf->seg_id, IPC_RMID, NULL);
274259

@@ -313,7 +298,7 @@ segment_attach(opal_shmem_ds_t *ds_buf)
313298
);
314299

315300
/* update returned base pointer with an offset that hides our stuff */
316-
return (ds_buf->seg_base_addr + sizeof(opal_shmem_seg_hdr_t));
301+
return ds_buf->seg_base_addr;
317302
}
318303

319304
/* ////////////////////////////////////////////////////////////////////////// */

0 commit comments

Comments
 (0)