Changeset View
Standalone View
lib/libvmmapi/vmmapi.c
Context not available. | |||||
* and the adjoining guard regions. | * and the adjoining guard regions. | ||||
*/ | */ | ||||
len = VM_MMAP_GUARD_SIZE + objsize + VM_MMAP_GUARD_SIZE; | len = VM_MMAP_GUARD_SIZE + objsize + VM_MMAP_GUARD_SIZE; | ||||
flags = MAP_PRIVATE | MAP_ANON | MAP_NOCORE | MAP_ALIGNED_SUPER; | flags = MAP_PRIVATE | MAP_ANON | MAP_NOCORE | MAP_ALIGNED_SUPER | ||||
kib: flags is single-use and thus pointless. | |||||
ptr = mmap(NULL, len, PROT_NONE, flags, -1, 0); | | MAP_FIXED; | ||||
ptr = mmap(NULL, len, PROT_NONE, MAP_GUARD | MAP_ALIGNED_SUPER, | |||||
-1, 0); | |||||
if (ptr == MAP_FAILED) | if (ptr == MAP_FAILED) | ||||
return (-1); | return (-1); | ||||
Done Inline ActionsYou don't need this second call to mmap(). setup_memory_segment() maps the appropriate regions of memory via mmap() with MAP_FIXED below. You should just keep the existing line that does 'baseaddr = ptr + VM_MMAP_GUARD_SIZE;'. jhb: You don't need this second call to mmap(). setup_memory_segment() maps the appropriate regions… | |||||
Done Inline ActionsAh, okay. I didn't realize setup_memory_segment did that. I'll remove this call to mmap. lattera-gmail.com: Ah, okay. I didn't realize setup_memory_segment did that. I'll remove this call to mmap. | |||||
baseaddr = ptr + VM_MMAP_GUARD_SIZE; | baseaddr = mmap(ptr + VM_MMAP_GUARD_SIZE, objsize, PROT_NONE, | ||||
flags, -1, 0); | |||||
if (baseaddr == MAP_FAILED) { | |||||
munmap(ptr, len); | |||||
kibUnsubmitted Done Inline ActionsI munmap(2) fails, the interesting errno value is obliterated. kib: I munmap(2) fails, the interesting errno value is obliterated. | |||||
return (-1); | |||||
} | |||||
if (ctx->highmem > 0) { | if (ctx->highmem > 0) { | ||||
gpa = 4*GB; | gpa = 4*GB; | ||||
len = ctx->highmem; | len = ctx->highmem; | ||||
Done Inline ActionsI would probably leave the existing comment as it is already accurate. I would also probably leave the len2 helper variable to minimize the diff. jhb: I would probably leave the existing comment as it is already accurate. I would also probably… | |||||
Done Inline ActionsI'll add the comment back in. As far as len2 is concerned, I figured that since kib complained about the already-existing flags variable being single-use, I'd also get rid of len2, since it, too, is single-use. I can revert that change, though. lattera-gmail.com: I'll add the comment back in.
As far as len2 is concerned, I figured that since kib complained… | |||||
Context not available. | |||||
* adjoining guard regions. | * adjoining guard regions. | ||||
*/ | */ | ||||
len2 = VM_MMAP_GUARD_SIZE + len + VM_MMAP_GUARD_SIZE; | len2 = VM_MMAP_GUARD_SIZE + len + VM_MMAP_GUARD_SIZE; | ||||
flags = MAP_PRIVATE | MAP_ANON | MAP_NOCORE | MAP_ALIGNED_SUPER; | flags = MAP_PRIVATE | MAP_ANON | MAP_NOCORE | MAP_ALIGNED_SUPER | ||||
kibUnsubmitted Done Inline ActionsAgain flags use is pointless. Before your patch, it was used to avoid line wrapping in the mmap() line, most likely. kib: Again flags use is pointless. Before your patch, it was used to avoid line wrapping in the… | |||||
base = mmap(NULL, len2, PROT_NONE, flags, -1, 0); | | MAP_FIXED; | ||||
/* | |||||
* Map the entire region with MAP_GUARD first. Remap the | |||||
* device memory post-guard. | |||||
*/ | |||||
base = mmap(NULL, len2, PROT_NONE, | |||||
kibUnsubmitted Done Inline ActionsWeird line space use. kib: Weird line space use. | |||||
MAP_GUARD | MAP_ALIGNED_SUPER, -1, 0); | |||||
if (base == MAP_FAILED) | if (base == MAP_FAILED) | ||||
goto done; | goto done; | ||||
if (mmap(base + VM_MMAP_GUARD_SIZE, len, PROT_NONE, flags, -1, | |||||
0) == MAP_FAILED) { | |||||
munmap(base, len2); | |||||
goto done; | |||||
} | |||||
flags = MAP_SHARED | MAP_FIXED; | flags = MAP_SHARED | MAP_FIXED; | ||||
if ((ctx->memflags & VM_MEM_F_INCORE) == 0) | if ((ctx->memflags & VM_MEM_F_INCORE) == 0) | ||||
Context not available. |
flags is single-use and thus pointless.