Repository navigation
Conversation
| */ | ||
| mk_arch_send_ipi(target); | ||
| if (ret == -ENOSPC) | ||
| pr_warn_ratelimited("multikernel: IPI ring full for instance %d\n", |
There was a problem hiding this comment.
The code this replaces used printk_deferred for the ring-full case, with a comment explaining that console output reaches this path. That is still true: mktty_spawn_console_write() runs as a console ->write callback during printk flushing, holding mktty_console_lock with IRQs off, and calls multikernel_send_ipi_data(). If publish fails here, these plain printks recursively enter console flushing, which re-enters mktty_spawn_console_write() and self-deadlocks on mktty_console_lock. Ring full is most likely precisely during heavy console output, so this fires under exactly the wrong conditions. These messages should stay printk_deferred (ratelimiting can be kept via __ratelimit).
| } | ||
| rc = mk_arch_spawn_instance(mk_image, instance, cpu); | ||
| if (rc == 0) { | ||
| rc = mk_ipi_shared_wait_ready(instance->ipi_data, mk_id, |
There was a problem hiding this comment.
This waits up to 120 seconds while still holding the kexec lock taken by kexec_trylock() at the top of multikernel_kexec_by_id(). While a wedged spawn is timing out, every other kexec operation returns -EBUSY, and if the host panics in that window __crash_kexec() fails its trylock, so the crash kernel never boots and the vmcore is lost. Consider dropping the lock before the wait (the new MK_STATE_LOADED check already excludes concurrent exec of the same instance), or at least a much smaller default timeout.
| ret = mk_ipi_shared_validate(instance->ipi_data); | ||
| if (ret) | ||
| return ret; | ||
| if (!atomic_read_acquire(&instance->ipi_data->ready)) |
There was a problem hiding this comment.
Behavior change worth confirming: previously the host could publish into the spawn ring while the spawn booted, and the spawn drained everything after init. Now sends fail with -EAGAIN until the spawn multikernel_init (subsys_initcall) marks the downlink ready, and the callers in messaging.c and mktty do not retry, so anything sent in that window is silently dropped. If this fail-closed behavior is intended it deserves a mention in the commit message; otherwise callers need a retry, or the ready gate needs to allow queuing during boot.
| if (ret) | ||
| mk_manifest_reject_and_park(ret); | ||
| if (!atomic_read_acquire(&host_instance->ipi_data->ready) || | ||
| READ_ONCE(host_instance->ipi_data->ready_instance_id) != 0) |
There was a problem hiding this comment.
This requires the host ring ready_instance_id to be exactly 0, but a mid-level kernel acting as a host marks its receive ring with its own root_instance->id, which is nonzero for a spawn. A nested spawn therefore always parks here with -EHOSTDOWN. Nesting already has known gaps, but this turns them into a hard block at the handshake.
| return ret; | ||
| } | ||
|
|
||
| ret = mk_ipi_shared_mark_ready(root_instance->ipi_data, |
There was a problem hiding this comment.
If mk_instance_restore_from_manifest() fails to allocate the root instance (its -ENOMEM path returns with root_instance still NULL and does not park), this dereferences NULL at subsys_initcall time. A NULL check turning it into a clean init failure would be safer.
3330928 to
1a984b7
Compare
|
Why This grows the vmlinux Note also that an older vmlinux with the 8-byte note now silently falls through to "entry not found" instead of a clear mismatch error like the bzImage path gives. More generally the design looks over-complicated: the 64-bit owner+index producer gate with five slot states and cancel/recover logic, and the host-side 120s readiness poll with abort-on-timeout, could both be replaced by a plain per-slot ready flag plus "reset the ring after confirm_parked", which is what the old |
1a984b7 to
4a71019
Compare
4a71019 to
e19b6b7
Compare
|
@nickolaev Both the PR description and the commit message still contain "timer calibration". Also, there is a merge conflict. |
e19b6b7 to
a7a784e
Compare
|
@nickolaev Please keep this PR to the duplex parent/child link fix (per-link rings, single producer, reset after confirm_parked, routing to the real parent) and drop the unrelated changes:
Send any of these separately if still needed. Please also update the PR title and description. |
Children currently publish into a shared parent inbox, while hardirq sends depend on mutex-backed instance lookup. Give each parent/child pair separate rings and serialize local producers in each direction. Use RCU for endpoint and handler lifetime. Keep callbacks in hardirq, bound each dispatch pass to eight links and eight slots per link, and scan readiness for fair continuation. Enable dispatch after built-in consumers register so messages can queue during boot. Preserve cursors across detach and reset only the selected link after confirmed park. Route to the manifest's parent ID, map its halt area separately, and order publication before the APIC doorbell. Validate image and endpoint publication under mk_instance_mutex against the referenced instance so removal and ID reuse cannot attach a stale link. Signed-off-by: Nikolay Nikolaev <nicknickolaev@gmail.com>
a7a784e to
957fb0b
Compare
|
I am very concerned about Publishing a kimage is a kexec job: set This function ties the two together so the endpoint can be registered at load time, and that coupling causes real problems:
None of the problems this PR fixes (single producer per ring, halt notices to the real parent, RCU handler lifetime) need anything at load time. Suggested change:
The destroy vs. load ID reuse check ( Minor: a publish failure returns without any |
Give each parent/child pair separate IPI rings and serialize local producers in each direction. Preserve queue cursors across detach, reset the selected link after confirmed park, and route messages to the parent recorded in the manifest.
Protect endpoint and handler lifetime with RCU. Keep receive callbacks in hardirq, with at most eight links and eight slots per link in each dispatch pass. Enable dispatch after built-in consumers register so boot-time messages can queue. Validate image/endpoint publication against the referenced instance under the instance mutex.
Parent and child kernels must use the same transport layout; ABI negotiation and SR-IOV changes are outside this PR.