-
Notifications
You must be signed in to change notification settings - Fork 471
Fix building on armv7 #155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -24,6 +24,10 @@ | |||
#define DISPATCH_IO_DEBUG DISPATCH_DEBUG | |||
#endif | |||
|
|||
#ifndef PAGE_SIZE | |||
#define PAGE_SIZE 4096 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PAGE_SIZE is in <sys/user.h>
on linux I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup just checked, it's where it is, don't ask me how I can ever remember things like that...
it should be added to internal.h explicitly, for all platforms this is fine, BSDs have it too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very interesting. PAGE_SIZE is not defined in sys/user.h on arm. It's referenced in a few places, but I can't see how it's defined in those cases.
wdillon@tegra-ubuntu /usr/include ack-grep "PAGE_SIZE"
scsi/sg.h
188:#define SG_SCATTER_SZ (8 * 4096) /* PAGE_SIZE not available to user */
190: The value must be a power of 2 and <= (PAGE_SIZE * 32) [131072 bytes on
191: i386]. The minimum value is PAGE_SIZE. If scatter-gather not supported
194: PAGE_SIZE by calling getpagesize() defined in unistd.h . */
elf.h
1882:#define PF_HP_PAGE_SIZE 0x00100000
arm-linux-gnueabihf/bits/xopen_lim.h
50: PAGE_SIZE Size of bytes of a page.
arm-linux-gnueabihf/bits/confname.h
135:#define _SC_PAGE_SIZE _SC_PAGESIZE
asm-generic/shmparam.h
4:#define SHMLBA PAGE_SIZE /* attach addr a multiple of this */
linux/resource.h
71:#define MLOCK_LIMIT ((PAGE_SIZE > 64*1024) ? PAGE_SIZE : 64*1024)
linux/vhost.h
58:#define VHOST_PAGE_SIZE 0x1000
linux/kvm.h
352: ((PAGE_SIZE - sizeof(struct kvm_coalesced_mmio_ring)) / \
499:#define KVM_PPC_PAGE_SIZES_MAX_SZ 8
509: struct kvm_ppc_one_page_size enc[KVM_PPC_PAGE_SIZES_MAX_SZ];
512:#define KVM_PPC_PAGE_SIZES_REAL 0x00000001
519: struct kvm_ppc_one_seg_page_size sps[KVM_PPC_PAGE_SIZES_MAX_SZ];
linux/binfmts.h
14:#define MAX_ARG_STRLEN (PAGE_SIZE * 32)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then you should use getpagesize() or sysconf(_SC_PAGESIZE) for the define. I assume arm doesn't define it because there are both 4k and 16k devices out there and it's not tied to the ABI.
Thanks, @MadCoder ! |
Fix building on armv7 Signed-off-by: Daniel A. Steffen <dsteffen@apple.com>
This patch enables compilation from build-script on armv7. I don't expect this patch to be without controversy; rather I'd like this to be the opening point for some discussion about the specific implementation.
In particular, I'd like to draw attention to this PR at foundation: https://github.com/apple/swift-corelibs-foundation/pull/399/files which addresses val_list in the same way as presented here.
Furthermore, PAGE_SIZE is, for some reason, undefined on ARM. The definition provided here is an assumption, and kind of a hack. I'd happy to receive some guidance whether there is a better way to move forward.
Thanks!