Skip to content

Commit 5ed9fd2

Browse files
wenningerkwenningerk
committed
Merge pull request #134 from wenningerk/aligned_alloc
Fix: avoid using deprecated valloc & frequent aligned alloc
2 parents f4ca41f + 9e6cbba commit 5ed9fd2

3 files changed

Lines changed: 84 additions & 26 deletions

File tree

‎src/sbd-md.c‎

Lines changed: 67 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,16 @@ char2cmd(const char cmd)
8989
static void
9090
close_device(struct sbd_context *st)
9191
{
92-
close(st->devfd);
92+
if (!st) {
93+
return;
94+
}
95+
if (st->ioctx) {
96+
io_destroy(st->ioctx);
97+
}
98+
if (st->devfd >= 0) {
99+
close(st->devfd);
100+
}
101+
free(st->buffer);
93102
free(st);
94103
}
95104

@@ -101,15 +110,15 @@ open_device(const char* devname, int loglevel)
101110
if (!devname)
102111
return NULL;
103112

104-
st = malloc(sizeof(struct sbd_context));
105-
if (!st)
113+
st = calloc(1, sizeof(struct sbd_context));
114+
if (!st) {
106115
return NULL;
107-
memset(st, 0, sizeof(struct sbd_context));
116+
}
117+
st->devfd = -1;
108118

109119
if (io_setup(1, &st->ioctx) != 0) {
110120
cl_perror("io_setup failed");
111-
free(st);
112-
return NULL;
121+
goto out;
113122
}
114123

115124
st->devfd = open(devname, O_SYNC|O_RDWR|O_DIRECT);
@@ -120,31 +129,37 @@ open_device(const char* devname, int loglevel)
120129
} else {
121130
cl_log(loglevel, "Opening device %s failed.", devname);
122131
}
123-
free(st);
124-
return NULL;
132+
goto out;
125133
}
126134

127135
ioctl(st->devfd, BLKSSZGET, &sector_size);
128136

129137
if (sector_size == 0) {
130138
cl_perror("Get sector size failed.\n");
131-
close_device(st);
132-
return NULL;
139+
goto out;
140+
}
141+
142+
if (posix_memalign(&st->buffer, sector_size, sector_size)) {
143+
cl_perror("Couldn't allocate sector-buffer.");
144+
goto out;
133145
}
134146

135147
return st;
148+
149+
out:
150+
close_device(st);
151+
return NULL;
136152
}
137153

138154
static void *
139155
sector_alloc(void)
140156
{
141157
void *x;
142158

143-
x = valloc(sector_size);
159+
x = calloc(1, sector_size);
144160
if (!x) {
145161
exit(1);
146162
}
147-
memset(x, 0, sector_size);
148163

149164
return x;
150165
}
@@ -162,9 +177,11 @@ sector_io(struct sbd_context *st, int sector, void *data, int rw)
162177

163178
memset(&st->io, 0, sizeof(struct iocb));
164179
if (rw) {
165-
io_prep_pwrite(&st->io, st->devfd, data, sector_size, (long long) sector_size * sector);
180+
memcpy(st->buffer, data, sector_size);
181+
io_prep_pwrite(&st->io, st->devfd, st->buffer, sector_size, (long long) sector_size * sector);
166182
} else {
167-
io_prep_pread(&st->io, st->devfd, data, sector_size, (long long) sector_size * sector);
183+
memset(st->buffer, 0, sector_size);
184+
io_prep_pread(&st->io, st->devfd, st->buffer, sector_size, (long long) sector_size * sector);
168185
}
169186

170187
if (io_submit(st->ioctx, 1, ios) != 1) {
@@ -179,7 +196,7 @@ sector_io(struct sbd_context *st, int sector, void *data, int rw)
179196
cl_log(LOG_ERR, "Failed to retrieve IO events (rw=%d)", rw);
180197
return -1;
181198
} else if (r < 1L) {
182-
cl_log(LOG_INFO, "Cancelling IO request due to timeout (rw=%d)", rw);
199+
cl_log(LOG_INFO, "Cancelling IO request due to timeout (rw=%d, r=%ld)", rw, r);
183200
r = io_cancel(st->ioctx, ios[0], &event);
184201
if (r) {
185202
DBGLOG(LOG_INFO, "Could not cancel IO request (rw=%d)", rw);
@@ -195,6 +212,9 @@ sector_io(struct sbd_context *st, int sector, void *data, int rw)
195212

196213
/* IO is happy */
197214
if (event.res == sector_size) {
215+
if (!rw) {
216+
memcpy(data, st->buffer, sector_size);
217+
}
198218
return 0;
199219
} else {
200220
cl_log(LOG_ERR, "Short IO (rw=%d, res=%lu, sector_size=%d)",
@@ -322,11 +342,13 @@ header_get(struct sbd_context *st)
322342

323343
if (header_read(st, s_header) < 0) {
324344
cl_log(LOG_ERR, "Unable to read header from device %d", st->devfd);
345+
free(s_header);
325346
return NULL;
326347
}
327348

328349
if (valid_header(s_header) < 0) {
329350
cl_log(LOG_ERR, "header on device %d is not valid.", st->devfd);
351+
free(s_header);
330352
return NULL;
331353
}
332354

@@ -364,6 +386,8 @@ header_dump(struct sbd_context *st)
364386
(unsigned long)s_header->timeout_loop);
365387
printf("Timeout (msgwait) : %lu\n",
366388
(unsigned long)s_header->timeout_msgwait);
389+
390+
free(s_header);
367391
return 0;
368392
}
369393

@@ -916,6 +940,7 @@ get_first_msgwait(struct servants_list_item *servants)
916940
if (s_header != NULL) {
917941
msgwait = (unsigned long)s_header->timeout_msgwait;
918942
close_device(st);
943+
free(s_header);
919944
return msgwait;
920945
}
921946

@@ -1081,13 +1106,15 @@ int servant_md(const char *diskname, int mode, const void* argp)
10811106
s_header = header_get(st);
10821107
if (!s_header) {
10831108
cl_log(LOG_ERR, "Not a valid header on %s", diskname);
1084-
exit(EXIT_MD_SERVANT_IO_FAIL);
1109+
rc = EXIT_MD_SERVANT_IO_FAIL;
1110+
goto out;
10851111
}
10861112

10871113
if (servant_check_timeout_inconsistent(s_header) < 0) {
10881114
cl_log(LOG_ERR, "Timeouts on %s do not match first device",
10891115
diskname);
1090-
exit(EXIT_MD_SERVANT_IO_FAIL);
1116+
rc = EXIT_MD_SERVANT_IO_FAIL;
1117+
goto out;
10911118
}
10921119

10931120
if (s_header->minor_version > 0) {
@@ -1107,7 +1134,8 @@ int servant_md(const char *diskname, int mode, const void* argp)
11071134
if (slot_read(st, mbox, s_node) < 0) {
11081135
cl_log(LOG_ERR, "Unable to read node entry on %s",
11091136
diskname);
1110-
exit(EXIT_MD_SERVANT_IO_FAIL);
1137+
rc = EXIT_MD_SERVANT_IO_FAIL;
1138+
goto out;
11111139
}
11121140

11131141
cl_log(LOG_NOTICE, "Monitoring slot %d on disk %s", mbox, diskname);
@@ -1168,28 +1196,36 @@ int servant_md(const char *diskname, int mode, const void* argp)
11681196
s_header_retry = header_get(st);
11691197
if (!s_header_retry) {
11701198
cl_log(LOG_ERR, "No longer found a valid header on %s", diskname);
1171-
exit(EXIT_MD_SERVANT_IO_FAIL);
1199+
rc = EXIT_MD_SERVANT_IO_FAIL;
1200+
goto out;
11721201
}
11731202
if (memcmp(s_header, s_header_retry, sizeof(*s_header)) != 0) {
11741203
cl_log(LOG_ERR, "Header on %s changed since start-up!", diskname);
1175-
exit(EXIT_MD_SERVANT_IO_FAIL);
1204+
free(s_header_retry);
1205+
rc = EXIT_MD_SERVANT_IO_FAIL;
1206+
goto out;
11761207
}
11771208
free(s_header_retry);
11781209

11791210
s_node_retry = sector_alloc();
11801211
if (slot_read(st, mbox, s_node_retry) < 0) {
11811212
cl_log(LOG_ERR, "slot read failed in servant.");
1182-
exit(EXIT_MD_SERVANT_IO_FAIL);
1213+
free(s_node_retry);
1214+
rc = EXIT_MD_SERVANT_IO_FAIL;
1215+
goto out;
11831216
}
11841217
if (memcmp(s_node, s_node_retry, sizeof(*s_node)) != 0) {
11851218
cl_log(LOG_ERR, "Node entry on %s changed since start-up!", diskname);
1186-
exit(EXIT_MD_SERVANT_IO_FAIL);
1219+
free(s_node_retry);
1220+
rc = EXIT_MD_SERVANT_IO_FAIL;
1221+
goto out;
11871222
}
11881223
free(s_node_retry);
11891224

11901225
if (mbox_read(st, mbox, s_mbox) < 0) {
11911226
cl_log(LOG_ERR, "mbox read failed in servant.");
1192-
exit(EXIT_MD_SERVANT_IO_FAIL);
1227+
rc = EXIT_MD_SERVANT_IO_FAIL;
1228+
goto out;
11931229
}
11941230

11951231
if (s_mbox->cmd > 0) {
@@ -1204,14 +1240,17 @@ int servant_md(const char *diskname, int mode, const void* argp)
12041240
sigqueue(ppid, SIG_TEST, signal_value);
12051241
break;
12061242
case SBD_MSG_RESET:
1207-
exit(EXIT_MD_SERVANT_REQUEST_RESET);
1243+
rc = EXIT_MD_SERVANT_REQUEST_RESET;
1244+
goto out;
12081245
case SBD_MSG_OFF:
1209-
exit(EXIT_MD_SERVANT_REQUEST_SHUTOFF);
1246+
rc = EXIT_MD_SERVANT_REQUEST_SHUTOFF;
1247+
goto out;
12101248
case SBD_MSG_EXIT:
12111249
sigqueue(ppid, SIG_EXITREQ, signal_value);
12121250
break;
12131251
case SBD_MSG_CRASHDUMP:
1214-
exit(EXIT_MD_SERVANT_REQUEST_CRASHDUMP);
1252+
rc = EXIT_MD_SERVANT_REQUEST_CRASHDUMP;
1253+
goto out;
12151254
default:
12161255
/* FIXME:
12171256
An "unknown" message might result
@@ -1240,6 +1279,8 @@ int servant_md(const char *diskname, int mode, const void* argp)
12401279
}
12411280
}
12421281
out:
1282+
free(s_header);
1283+
free(s_node);
12431284
free(s_mbox);
12441285
close_device(st);
12451286
exit(rc);

‎src/sbd.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ struct sbd_context {
108108
int devfd;
109109
io_context_t ioctx;
110110
struct iocb io;
111+
void *buffer;
111112
};
112113

113114
enum pcmk_health

‎tests/sbd-testbed.c‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ typedef int (*orig_close_f_type)(int fd);
6363
typedef FILE *(*orig_fopen_f_type)(const char *pathname, const char *mode);
6464
typedef int (*orig_fclose_f_type)(FILE *fp);
6565
typedef int (*orig_io_setup_f_type)(int nr_events, io_context_t *ctx_idp);
66+
typedef int (*orig_io_destroy_f_type)(io_context_t ctx_id);
6667
typedef int (*orig_io_submit_f_type)(io_context_t ctx_id, long nr, struct iocb *ios[]);
6768
typedef int (*orig_io_getevents_f_type)(io_context_t ctx_id, long min_nr, long nr,
6869
struct io_event *events, struct timespec *timeout);
@@ -93,6 +94,7 @@ static orig_close_f_type orig_close = NULL;
9394
static orig_fopen_f_type orig_fopen = NULL;
9495
static orig_fclose_f_type orig_fclose = NULL;
9596
static orig_io_setup_f_type orig_io_setup = NULL;
97+
static orig_io_destroy_f_type orig_io_destroy = NULL;
9698
static orig_io_submit_f_type orig_io_submit = NULL;
9799
static orig_io_getevents_f_type orig_io_getevents = NULL;
98100
static orig_io_cancel_f_type orig_io_cancel = NULL;
@@ -158,6 +160,7 @@ init (void)
158160
exit(1);
159161
}
160162
orig_io_setup = (orig_io_setup_f_type)dlsym_fatal(handle,"io_setup");
163+
orig_io_destroy = (orig_io_destroy_f_type)dlsym_fatal(handle,"io_destroy");
161164
orig_io_submit = (orig_io_submit_f_type)dlsym_fatal(handle,"io_submit");
162165
orig_io_getevents = (orig_io_getevents_f_type)dlsym_fatal(handle,"io_getevents");
163166
orig_io_cancel = (orig_io_cancel_f_type)dlsym_fatal(handle,"io_cancel");
@@ -625,6 +628,19 @@ int io_setup(int nr_events, io_context_t *ctx_idp)
625628
return 0;
626629
}
627630

631+
int io_destroy(io_context_t ctx_id)
632+
{
633+
init();
634+
635+
if (!translate_aio) {
636+
return orig_io_destroy(ctx_id);
637+
}
638+
639+
if (ctx_id != &our_io_context) {
640+
return EINVAL;
641+
}
642+
return 0;
643+
}
628644

629645
int io_submit(io_context_t ctx_id, long nr, struct iocb *ios[])
630646
{

0 commit comments

Comments
 (0)