Skip to content

Conversation

@gypsephi
Copy link
Contributor

#211
tested on qemu

@gypsephi gypsephi requested review from kubanrob and rottaran January 20, 2022 10:07
@gypsephi gypsephi self-assigned this Jan 20, 2022
Copy link
Collaborator

@kubanrob kubanrob left a comment

Choose a reason for hiding this comment

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

Tested with IHK. Maybe @rottaran has an opinion on the API/interface of ISchedulable and the EC. Looks fine otherwise.


virtual void saveState() = 0;

virtual bool hasPriority() { return false; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe at least the interface should be able to hande different priorities?
We could define some integer constants like HIGH_PRIORITY, DEFAULT_PRIORITY and have defined behaviour only for these. I imagine we want to have LOW_PRIORITY somewhen in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added comment.
integer constants sound good, but I don't think it is necessary.

Comment on lines +129 to +134
struct SetPriority : public InvocationBase {
constexpr static uint16_t label = (proto<<8) + SET_PRIORITY;
SetPriority(bool priority)
: InvocationBase(label,getLength(this)), priority(priority) {}
bool priority;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if we should limit that to a bool. See comment on interface. setPriority(false) could be hard to interpret.

// or if if current ec got ready in case of race condition
if (current == nullptr || current == ec) home->preempt();
// wake up the hardware thread if it has no execution context running
// or if if current ec got ready in case of race condition
Copy link
Collaborator

Choose a reason for hiding this comment

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

If a running EC priority has been set to default from another hw thread (-> without interrupting it), this should also be enough to prevent the EC from running if there is another high priority EC waiting to run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, but I don't want to handle this special case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants