locking heap wrapper #1337
Merged
locking heap wrapper #1337
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
In init_pagecache(), pc->completions is initialized as an objcache, and is not locked. It should be locked instead (there is also an XXX comment that says this at pagecache.c:1320), because completions can be allocated and deallocated from that objcache without the kernel lock.
|
In platform/pc/service.c, offset_block_io() calls allocate_merge() on the general kernel heap; since this can be called without the kernel lock (e.g. via pagecache_write_sg_finish -> filesystem_storage_write -> write_extent), it should use a locked heap instead. |
|
I addressed the errors @francescolavra found and removed the unnecessary size parameter from locking_heap_wrapper. |
…tions to locked heap locking_heap under src/kernel; use locking for sg_init, init_pagecache and virtio net; SMP_ENABLE by default for CI; host filesystems on locked heap; pagecache_write_sg_finish should not update write_count and apply completions on a read error; put xen grant_entry heap on locked
…ecessary size parameter from locking_heap_wrapper, fill meta field; use locking wrapper on pagecache completion objcache; offset_block_io: allocate merge off of locked heap; revert SMP_ENABLE for merge into master
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As enumerated in #1302, there are many paths in the kernel where the kernel lock is not held and yet allocations or deallocations are being made from one of the kernel heaps. To address this issue, these changes introduce a "locked" kernel heap, using a wrapper heap which wraps heap methods with spinlock aquires, which helps to make such paths safe.
As the interface for the id heap is fairly complex and wouldn't be covered by a generic heap wrapper, a "locking" flag is specified on ID heap creation. The virtual huge and page heaps are now locking and thus safe to use from any context. The "backed" heap uses the locking wrapper, and a "locked" variant of general (using another mcache instance) is created for general-purpose allocations outside of the kernel lock. Allocations made under protection of the kernel lock should continue to use the general heap, as this will avoid unnecessary spinlock operations.
Note: SMP is enabled temporarily in this branch just to run through CI - it will be removed before merging (and until we agree it's safe to enable by default).