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

Implement SHM region management and re-enable DAX in virtiofs #212

Merged
merged 7 commits into from
Sep 5, 2024

Conversation

slp
Copy link
Contributor

@slp slp commented Aug 9, 2024

We have multiple devices (fs and gpu) that can use SHM regions. So far,
we were creating a single SHM region so only one device could make use
of it. In this commit, we implement SHM region management so multiple
devices can request their own regions.

For the moment, we're hardcoding the SHM region sizes for gpu and fs. A
future commit will extend the API so users can configure those sizes as
desired.

@slp slp force-pushed the generalize-shm branch 6 times, most recently from aae7810 to 3761190 Compare August 9, 2024 10:51
@slp
Copy link
Contributor Author

slp commented Aug 9, 2024

cc/ @asahilina

@rhatdan
Copy link
Member

rhatdan commented Aug 19, 2024

@tylerfanelli Can you review this?

@tylerfanelli
Copy link
Collaborator

@slp Any way I can test?

@slp
Copy link
Contributor Author

slp commented Aug 21, 2024

@tylerfanelli yes, with this commit in and building with gpu, in the guest you should see:

  • virtio-gpu gets a memory window:
[    1.191469] [drm] Host memory window: 0x280000000 +0x200000000
  • each virtio-fs device gets its own DAX window:
[    1.982987] virtiofs virtio4: Cache len: 0x40000000 @ 0x700000000
[    1.989999] virtiofs virtio5: Cache len: 0x40000000 @ 0x740000000
  • mounting the virtio-fs volume with DAX should work (even on macOS!), and reading files should be possible:
slp@localhost:~$ sudo mount -t virtiofs -o dax test /mnt
slp@localhost:~$ dd if=/mnt/mistral-7b-instruct-v0.2.Q4_0.gguf of=/dev/null bs=128k
31348+1 records in
31348+1 records out
4108917024 bytes (4.1 GB, 3.8 GiB) copied, 1.23938 s, 3.3 GB/s

@slp slp marked this pull request as draft August 23, 2024 07:24
@slp
Copy link
Contributor Author

slp commented Aug 23, 2024

Let's put this on hold until we figure out what's happing in #213 (comment)

slp added 3 commits August 28, 2024 16:00
Signed-off-by: Sergio Lopez <[email protected]>
Remove broken and useless kvm_context test.

Signed-off-by: Sergio Lopez <[email protected]>
In a future commit we need to extend the functionality of
create_guest_memory, so take this opportunity to unify it across
flavors/variants.

Signed-off-by: Sergio Lopez <[email protected]>
@slp slp force-pushed the generalize-shm branch 4 times, most recently from a8719b5 to 4882a7e Compare August 28, 2024 14:33
slp added 3 commits August 29, 2024 14:19
We have multiple devices (fs and gpu) that can use SHM regions. So far,
we were creating a single SHM region so only one device could make use
of it. In this commit, we implement SHM region management so multiple
devices can request their own regions.

For the moment, we're hardcoding the SHM region sizes for gpu and fs. A
future commit will extend the API so users can configure those sizes as
desired.

Signed-off-by: Sergio Lopez <[email protected]>
After splitting the functionality into a worker, the SHM region was
left behind. Plumb it now to re-enable DAX support.

Signed-off-by: Sergio Lopez <[email protected]>
Bring virtiofs DAX support into macOS relying on the mechanisms added
for supporting virtio-gpu blobs, which allow us to request HVF the
injection of memory regions.

Signed-off-by: Sergio Lopez <[email protected]>
Introduce krun_add_virtiofs2 and krun_set_gpu_options2 as variants of
krun_add_virtiofs and krun_set_gpu_options respectively, to allow users
to specify the DAX/host SHM window size for the device.

We're adding variants of existing functions to avoid breaking existing
consumers of the library. This isn't great, but we'll do a huge clean up
of the API in 2.0.

Signed-off-by: Sergio Lopez <[email protected]>
@slp slp marked this pull request as ready for review August 29, 2024 12:31
@slp
Copy link
Contributor Author

slp commented Aug 30, 2024

@tylerfanelli PTAL, should be ready now.

Copy link
Collaborator

@tylerfanelli tylerfanelli left a comment

Choose a reason for hiding this comment

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

LGTM on x86_64 Linux

@slp Side note, with this built and latest version of libkrunfw, the chroot_vm example is failing for me (x86_64 Linux). I'll submit an issue so we can track this. Can you confirm?

@tylerfanelli tylerfanelli merged commit 7b13b39 into containers:main Sep 5, 2024
5 checks passed
@slp
Copy link
Contributor Author

slp commented Sep 5, 2024

@tylerfanelli yes, please, open an issue detailing that.

@tylerfanelli
Copy link
Collaborator

Tracked with #216

@slp slp mentioned this pull request Sep 11, 2024
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.

3 participants