revise Hermit's mutex interface to support the behaviour of StaticMutex by stlankes · Pull Request #77610 · rust-lang/rust

stlankes · 2020-10-06T09:50:37Z

#77147 simplifies things by splitting this Mutex type into two types matching the two use cases: StaticMutex and MovableMutex. To support the new behavior of StaticMutex, we move part of the mutex implementation into libstd.

The interface to the OS changed. Consequently, I removed a few functions, which aren't longer needed.

rust-highfive · 2020-10-06T09:50:42Z

r? @LukasKalbertodt

(rust_highfive has picked a reviewer for you, use r? to override)

m-ou-se · 2020-10-07T19:41:53Z

new behavior of StaticMutex

The behaviour shouldn't have changed. I only gave it a name. Using sys::(hermit::)Mutex like this (without .init() and .destroy()), was already done by all of std's static Mutexes before #77147.

Did #77147 specifically break something for you? Or was this already a problem before?

stlankes · 2020-10-07T21:13:05Z

I recognize the problem just after your commit. Maybe it was already there before. In my old approach, StaticMutex' are sometimes not correctly initialized.

stlankes · 2020-10-09T06:28:02Z

My fix to pass the format check was wrong. I fixed it and it works on my systems.

rust-lang#77147 simplifies things by splitting this Mutex type into two types matching the two use cases: StaticMutex and MovableMutex. To support the behavior of StaticMutex, we move part of the mutex implementation into libstd.

m-ou-se · 2020-10-12T19:06:40Z

Does std compile for hermit now? Because it looks like type MovableMutex = Box<Mutex>; (or type MovableMutex = Mutex;) is missing for this platform in sys/hermit/mutex.rs.

stlankes · 2020-10-12T20:50:16Z

My local version seems to work. After an update, std was broken. I fixed it with my latest commits

stlankes · 2020-10-13T10:20:14Z

The latest commit isn't related to the Mutex interface. The alloc crate provides now a new symbol __rg_oom. Our kernel and also Hermit's "user space" based on this crate. To avoid duplicated symbols, I moved a version to our libos.

the commit avoid an alignement issue in Mutex implementation

stlankes · 2020-10-14T04:29:04Z

@m-ou-se When should I use type MovableMutex = Box<Mutex>;? If I don't used a boxed type, the kernel triggers some exceptions because a few data types aren't aligned.

By the way, the current version works and I created a docker container, which provide the nightly compiler for HermitCore.

m-ou-se · 2020-10-14T07:58:50Z

@m-ou-se When should I use type MovableMutex = Box<Mutex>;?

A Box should only be used there if a Mutex object may not be moved (while not borrowed/used), e.g. because it might have internal pointers. Posix pthread_mutex_t needs it. Most Mutex implementations don't need it and should use type MovableMutex = Mutex;.

If I don't used a boxed type, the kernel triggers some exceptions because a few data types aren't aligned.

That sounds like a type (Mutex, SpinLock, or PriorityQueue?) is missing a #[repr(align(..))].

stlankes · 2020-10-14T08:26:48Z

That sounds like a type (Mutex, SpinLock, or PriorityQueue?) is missing a #[repr(align(..))].

Good point, I will check it.

stlankes · 2020-10-15T03:26:48Z

That sounds like a type (Mutex, SpinLock, or PriorityQueue?) is missing a #[repr(align(..))].

Good point, I will check it.

No, that isn't the problem. I would say the current version is fine. @LukasKalbertodt If you like, you are able to review the PR.

stlankes · 2020-10-23T06:09:44Z

@dtolnay Do you have time to review the code?

m-ou-se · 2020-10-23T10:55:22Z

@bors r+

I did not review this in detail, as Hermit is a Tier 3 platform.

bors · 2020-10-23T10:55:23Z

📌 Commit bf268fe has been approved by m-ou-se

…as-schievink Rollup of 12 pull requests Successful merges: - rust-lang#75115 (`#[deny(unsafe_op_in_unsafe_fn)]` in sys/cloudabi) - rust-lang#76614 (change the order of type arguments on ControlFlow) - rust-lang#77610 (revise Hermit's mutex interface to support the behaviour of StaticMutex) - rust-lang#77830 (Simplify query proc-macros) - rust-lang#77930 (Do not ICE with TraitPredicates containing [type error]) - rust-lang#78069 (Fix const core::panic!(non_literal_str).) - rust-lang#78072 (Cleanup constant matching in exhaustiveness checking) - rust-lang#78119 (Throw core::panic!("message") as &str instead of String.) - rust-lang#78191 (Introduce a temporary for discriminant value in MatchBranchSimplification) - rust-lang#78272 (const_evaluatable_checked: deal with unused nodes + div) - rust-lang#78318 (TyCtxt: generate single impl block with `slice_interners` macro) - rust-lang#78327 (resolve: Relax macro resolution consistency check to account for any errors) Failed merges: r? `@ghost`

rust-highfive assigned LukasKalbertodt Oct 6, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 6, 2020

stlankes mentioned this pull request Oct 6, 2020

The current version requires changes in libstd hermit-os/hermit-rs#73

Closed

stlankes added 10 commits October 11, 2020 11:53

using the latest version of libhermit-rs

98fcc3f

revise code to pass the format check

d560b50

revise comments and descriptions of the helper functions

986c1fc

fix typos in new method

d6e955f

revise code to pass the format check

530f575

add hermit to the list of omit OS

8d8a290

remove obsolete function diverge

33fd08b

reuse implementation of the system provider "unsupported"

30c3dad

define required type 'MovableMutex'

1741e5b

stlankes force-pushed the dtors branch from 9bb67db to 1741e5b Compare October 12, 2020 20:48

move __rg_oom to the libos to avoid duplicated symbols

bc6b2ac

stlankes added 2 commits October 13, 2020 12:22

minor changes to pass the format check

77d9831

box mutex to get a movable mutex

bf268fe

the commit avoid an alignement issue in Mutex implementation

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 23, 2020

jonas-schievink mentioned this pull request Oct 23, 2020

Rollup of 11 pull requests #78282

Closed

This was referenced Oct 23, 2020

Rollup of 12 pull requests #78294

Closed

Rollup of 15 pull requests #78303

Closed

m-ou-se mentioned this pull request Oct 23, 2020

Make the hermit target typecheck #78304

Closed

jonas-schievink mentioned this pull request Oct 24, 2020

Rollup of 12 pull requests #78328

Closed

jonas-schievink mentioned this pull request Oct 24, 2020

Rollup of 12 pull requests #78334

Merged

bors merged commit e34263d into rust-lang:master Oct 25, 2020

rustbot added this to the 1.49.0 milestone Oct 25, 2020

This was referenced Oct 27, 2020

using compiler builtins to realize basic operations (e.g. memcpy) hermit-os/kernel#115

Merged

Build failed. hermit-os/hermit-rs-template#2

Closed

mkroening deleted the dtors branch December 11, 2022 19:09

Uh oh!

Conversation

stlankes commented Oct 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Oct 6, 2020

Uh oh!

m-ou-se commented Oct 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stlankes commented Oct 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stlankes commented Oct 9, 2020

Uh oh!

m-ou-se commented Oct 12, 2020

Uh oh!

stlankes commented Oct 12, 2020

Uh oh!

stlankes commented Oct 13, 2020

Uh oh!

stlankes commented Oct 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

m-ou-se commented Oct 14, 2020

Uh oh!

stlankes commented Oct 14, 2020

Uh oh!

stlankes commented Oct 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stlankes commented Oct 23, 2020

Uh oh!

m-ou-se commented Oct 23, 2020

Uh oh!

bors commented Oct 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

stlankes commented Oct 6, 2020 •

edited

Loading

m-ou-se commented Oct 7, 2020 •

edited

Loading

stlankes commented Oct 7, 2020 •

edited

Loading

stlankes commented Oct 14, 2020 •

edited

Loading

stlankes commented Oct 15, 2020 •

edited

Loading