Skip to content

Commit 4fbcf07

Browse files
Matt Helsleyrostedt
authored andcommitted
recordmcount: Clarify what cleanup() does
cleanup() mostly frees/unmaps the malloc'd/privately-mapped copy of the ELF file recordmcount is working on, which is set up in mmap_file(). It also deals with positioning within the pseduo prive-mapping of the file and appending to the ELF file. Split into two steps: mmap_cleanup() for the mapping itself file_append_cleanup() for allocations storing the appended ELF data. Also, move the global variable initializations out of the main, per-object-file loop and nearer to the alloc/init (mmap_file()) and two cleanup functions so we can more clearly see how they're related. Link: http://lkml.kernel.org/r/2a387ac86d133d22c68f57b9933c32bab1d09a2d.1564596289.git.mhelsley@vmware.com Signed-off-by: Matt Helsley <[email protected]> Signed-off-by: Steven Rostedt (VMware) <[email protected]>
1 parent c97fea2 commit 4fbcf07

File tree

1 file changed

+81
-70
lines changed

1 file changed

+81
-70
lines changed

scripts/recordmcount.c

Lines changed: 81 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -48,21 +48,26 @@ static void *file_map; /* pointer of the mapped file */
4848
static void *file_end; /* pointer to the end of the mapped file */
4949
static int file_updated; /* flag to state file was changed */
5050
static void *file_ptr; /* current file pointer location */
51+
5152
static void *file_append; /* added to the end of the file */
5253
static size_t file_append_size; /* how much is added to end of file */
5354

5455
/* Per-file resource cleanup when multiple files. */
55-
static void cleanup(void)
56+
static void file_append_cleanup(void)
57+
{
58+
free(file_append);
59+
file_append = NULL;
60+
file_append_size = 0;
61+
file_updated = 0;
62+
}
63+
64+
static void mmap_cleanup(void)
5665
{
5766
if (!mmap_failed)
5867
munmap(file_map, sb.st_size);
5968
else
6069
free(file_map);
6170
file_map = NULL;
62-
free(file_append);
63-
file_append = NULL;
64-
file_append_size = 0;
65-
file_updated = 0;
6671
}
6772

6873
/* ulseek, uwrite, ...: Check return value for errors. */
@@ -103,7 +108,8 @@ static ssize_t uwrite(void const *const buf, size_t const count)
103108
}
104109
if (!file_append) {
105110
perror("write");
106-
cleanup();
111+
file_append_cleanup();
112+
mmap_cleanup();
107113
return -1;
108114
}
109115
if (file_ptr < file_end) {
@@ -129,12 +135,76 @@ static void * umalloc(size_t size)
129135
void *const addr = malloc(size);
130136
if (addr == 0) {
131137
fprintf(stderr, "malloc failed: %zu bytes\n", size);
132-
cleanup();
138+
file_append_cleanup();
139+
mmap_cleanup();
133140
return NULL;
134141
}
135142
return addr;
136143
}
137144

145+
/*
146+
* Get the whole file as a programming convenience in order to avoid
147+
* malloc+lseek+read+free of many pieces. If successful, then mmap
148+
* avoids copying unused pieces; else just read the whole file.
149+
* Open for both read and write; new info will be appended to the file.
150+
* Use MAP_PRIVATE so that a few changes to the in-memory ElfXX_Ehdr
151+
* do not propagate to the file until an explicit overwrite at the last.
152+
* This preserves most aspects of consistency (all except .st_size)
153+
* for simultaneous readers of the file while we are appending to it.
154+
* However, multiple writers still are bad. We choose not to use
155+
* locking because it is expensive and the use case of kernel build
156+
* makes multiple writers unlikely.
157+
*/
158+
static void *mmap_file(char const *fname)
159+
{
160+
/* Avoid problems if early cleanup() */
161+
fd_map = -1;
162+
mmap_failed = 1;
163+
file_map = NULL;
164+
file_ptr = NULL;
165+
file_updated = 0;
166+
sb.st_size = 0;
167+
168+
fd_map = open(fname, O_RDONLY);
169+
if (fd_map < 0) {
170+
perror(fname);
171+
return NULL;
172+
}
173+
if (fstat(fd_map, &sb) < 0) {
174+
perror(fname);
175+
goto out;
176+
}
177+
if (!S_ISREG(sb.st_mode)) {
178+
fprintf(stderr, "not a regular file: %s\n", fname);
179+
goto out;
180+
}
181+
file_map = mmap(0, sb.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE,
182+
fd_map, 0);
183+
if (file_map == MAP_FAILED) {
184+
mmap_failed = 1;
185+
file_map = umalloc(sb.st_size);
186+
if (!file_map) {
187+
perror(fname);
188+
goto out;
189+
}
190+
if (read(fd_map, file_map, sb.st_size) != sb.st_size) {
191+
perror(fname);
192+
free(file_map);
193+
file_map = NULL;
194+
goto out;
195+
}
196+
} else
197+
mmap_failed = 0;
198+
out:
199+
close(fd_map);
200+
fd_map = -1;
201+
202+
file_end = file_map + sb.st_size;
203+
204+
return file_map;
205+
}
206+
207+
138208
static unsigned char ideal_nop5_x86_64[5] = { 0x0f, 0x1f, 0x44, 0x00, 0x00 };
139209
static unsigned char ideal_nop5_x86_32[5] = { 0x3e, 0x8d, 0x74, 0x26, 0x00 };
140210
static unsigned char *ideal_nop;
@@ -238,61 +308,6 @@ static int make_nop_arm64(void *map, size_t const offset)
238308
return 0;
239309
}
240310

241-
/*
242-
* Get the whole file as a programming convenience in order to avoid
243-
* malloc+lseek+read+free of many pieces. If successful, then mmap
244-
* avoids copying unused pieces; else just read the whole file.
245-
* Open for both read and write; new info will be appended to the file.
246-
* Use MAP_PRIVATE so that a few changes to the in-memory ElfXX_Ehdr
247-
* do not propagate to the file until an explicit overwrite at the last.
248-
* This preserves most aspects of consistency (all except .st_size)
249-
* for simultaneous readers of the file while we are appending to it.
250-
* However, multiple writers still are bad. We choose not to use
251-
* locking because it is expensive and the use case of kernel build
252-
* makes multiple writers unlikely.
253-
*/
254-
static void *mmap_file(char const *fname)
255-
{
256-
file_map = NULL;
257-
sb.st_size = 0;
258-
fd_map = open(fname, O_RDONLY);
259-
if (fd_map < 0) {
260-
perror(fname);
261-
return NULL;
262-
}
263-
if (fstat(fd_map, &sb) < 0) {
264-
perror(fname);
265-
goto out;
266-
}
267-
if (!S_ISREG(sb.st_mode)) {
268-
fprintf(stderr, "not a regular file: %s\n", fname);
269-
goto out;
270-
}
271-
file_map = mmap(0, sb.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE,
272-
fd_map, 0);
273-
mmap_failed = 0;
274-
if (file_map == MAP_FAILED) {
275-
mmap_failed = 1;
276-
file_map = umalloc(sb.st_size);
277-
if (!file_map) {
278-
perror(fname);
279-
goto out;
280-
}
281-
if (read(fd_map, file_map, sb.st_size) != sb.st_size) {
282-
perror(fname);
283-
free(file_map);
284-
file_map = NULL;
285-
goto out;
286-
}
287-
}
288-
out:
289-
close(fd_map);
290-
291-
file_end = file_map + sb.st_size;
292-
293-
return file_map;
294-
}
295-
296311
static int write_file(const char *fname)
297312
{
298313
char tmp_file[strlen(fname) + 4];
@@ -438,10 +453,11 @@ static void MIPS64_r_info(Elf64_Rel *const rp, unsigned sym, unsigned type)
438453

439454
static int do_file(char const *const fname)
440455
{
441-
Elf32_Ehdr *const ehdr = mmap_file(fname);
442456
unsigned int reltype = 0;
457+
Elf32_Ehdr *ehdr;
443458
int rc = -1;
444459

460+
ehdr = mmap_file(fname);
445461
if (!ehdr)
446462
goto out;
447463

@@ -577,7 +593,8 @@ static int do_file(char const *const fname)
577593

578594
rc = write_file(fname);
579595
out:
580-
cleanup();
596+
file_append_cleanup();
597+
mmap_cleanup();
581598
return rc;
582599
}
583600

@@ -620,12 +637,6 @@ int main(int argc, char *argv[])
620637
strcmp(file + (len - ftrace_size), ftrace) == 0)
621638
continue;
622639

623-
/* Avoid problems if early cleanup() */
624-
fd_map = -1;
625-
mmap_failed = 1;
626-
file_map = NULL;
627-
file_ptr = NULL;
628-
file_updated = 0;
629640
if (do_file(file)) {
630641
fprintf(stderr, "%s: failed\n", file);
631642
++n_error;

0 commit comments

Comments
 (0)