Skip to content
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

Add function to check buffer alignment #87

Merged
merged 3 commits into from
Mar 12, 2016
Merged

Conversation

djs55
Copy link
Member

@djs55 djs55 commented Mar 6, 2016

Some protocols require buffers to be aligned to particular boundaries, for example

  • Xen shared memory pages must be page-aligned
  • Linux O_DIRECT I/O buffers must be sector-aligned

This PR adds a simple check_alignment function (similar to check_bounds) which returns true if the data inside a struct (i.e. the first byte at the offset) has an address which is suitably aligned.

This will enable better sanity-checking and error-reporting in other Mirage libraries.

djs55 added 2 commits March 6, 2016 17:03
Some protocols require buffers to be aligned: for example

- Xen shared memory protocols require alignment to pages
- Linux O_DIRECT requires buffers to be aligned to the disk
  sector size

The new function `check_alignment cstr alignment` returns
`true` if the data at `cstr.off` has a memory address for which
`address mod alignment = 0`. This will allow implementations which
require aligned buffers to check for it, and fail appropriately
if given unaligned buffers.

Note this patch assumes that javascript does not have buffer
addresses or memory alignment requirements, so the js stubs
always returns an address of 0.

Signed-off-by: David Scott <dave.scott@docker.com>
These tests verify that we find the expected number of aligned
offsets within a couple of example buffers.

Signed-off-by: David Scott <dave.scott@docker.com>

let check_alignment t alignment =
let address = address_bigstring t.buffer in
Int64.(rem (add address (of_int t.off)) (of_int alignment) = 0L)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are an awful lot of minor heap allocations here... if this is in the fast path as an assert it will probably increase GC pressure

@djs55
Copy link
Member Author

djs55 commented Mar 7, 2016

Yeah good point, I'll respin with a C version.

Before this patch we returned the buffer address from C and then
calculated using OCaml's boxed Int64.t. This patch reduces minor
allocations by performing the calculation in the C stubs. This
makes the function suitable for use in an assert in a performance
sensitive path.

Signed-off-by: David Scott <dave.scott@docker.com>
djs55 added a commit that referenced this pull request Mar 12, 2016
Add function to check buffer alignment
@djs55 djs55 merged commit 6a2070b into mirage:master Mar 12, 2016
@djs55 djs55 deleted the alignment branch March 12, 2016 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants