Opened 13 months ago

Last modified 8 months ago

#24554 needs_revision enhancement

sched: Have per-scheduler type data in a channel_t

Reported by: dgoulet Owned by: dgoulet
Priority: Very High Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-sched, 034-triage-20180328, 034-removed-20180328
Cc: Actual Points:
Parent ID: #23993 Points:
Reviewer: Sponsor:

Description

Right now in channel_t, we have a sched_heap_idx and scheduler_state. Both are specific to the scheduler layer and are per-channel. The KIST scheduler also keeps data per-channel in a global state it keeps some other place.

Because of #23744, which is a ticket about the scheduler state not applying to both Vanilla and KIST, we need a way to have a interface that allows any "scheduler data per-channel" to be specific to a type of scheduler (KIST or Vanilla).

To achieve compartmentalization of whatever state the scheduler needs to keep, we need an interface that allows a specific scheduler to store data per-channel instead of in a global state which, in the case of KIST, would avoid two things:

  1. Huge amount of memory allocation at runtime that needs to be done by the main loop (this is currently a problem with KIST code).
  1. Avoid data duplication or/and synchronization problem between the channel subsystem and the scheduler subsystem.

Finally, by doing so, we can build an opaque interface for the specific scheduler information data structure in order to make them *ONLY* accessible from the per-type scheduler code. This means a builtin protection to prevent any other layer in the tor to change or access the channel's scheduler state.

Once we have this capability, we can go ahead and fix #23744 and move the sched_heap_idx and scheduler_state out of a channel object so ONLY the scheduler has access to those and be able to have different semantic for different scheduler type.

Child Tickets

Change History (14)

comment:1 Changed 13 months ago by dgoulet

Status: assignedneeds_review

See branch: ticket24554_033_01
OnionGit: https://oniongit.eu/dgoulet/tor/merge_requests/12

comment:2 Changed 13 months ago by dgoulet

Parent ID: #23993

comment:3 Changed 13 months ago by pastly

Reviewer: pastly

comment:4 Changed 12 months ago by pastly

Status: needs_reviewneeds_revision

Left comments on oniongit.

  • One easy to fix compile issue.
  • Some rambling thoughts about the consequences of ignoring a chan with invalid sched_info in the middle of a scheduling run.

Marking needs_revision for the compile issue.

comment:5 Changed 12 months ago by dgoulet

pastly did review and I fixed stuff. I'll rebase this on latest master and open a new merge request for upstream merge.

comment:6 Changed 12 months ago by dgoulet

Status: needs_revisionneeds_review

See branch: ticket24554_033_02.
Oniongit MR: https://oniongit.eu/dgoulet/tor/merge_requests/13

One thing to notice is that the top commit is actually the commit for #23579. It ain't very complicated, removes code duplication and lays the ground work for future tickets like #23744 and the work to move the scheduler state from the channel_t object into sched_info_t *only* for Vanilla because KIST doesn't need it.

This is a slow and complicated work towards getting the scheduler subsystem much simpler, robust and easier to test. All this is part of #23993 parent ticket.

comment:7 Changed 12 months ago by dgoulet

Reviewer: pastly

(Removing pastly since he did a first review and might not want to do a second ;)

comment:8 Changed 12 months ago by dgoulet

Quick note. Moving sched_heap_idx out of channel_t and into sched_info_t is a bit more difficult because we use this offsetof() strategy with priority list so sched_heap_idx needs to offset from the channel_t object which is what is stored in the list. One possible solution would be to keep a pointer to the channel inside the sched_info_t and store those objects instead.

comment:9 Changed 12 months ago by dgoulet

Status: needs_reviewneeds_revision

I need to run this a bit more on top of #24665 and #24666.

comment:10 Changed 11 months ago by dgoulet

Priority: MediumVery High

comment:11 Changed 11 months ago by dgoulet

Milestone: Tor: 0.3.3.x-finalTor: 0.3.4.x-final

comment:12 Changed 9 months ago by nickm

Keywords: 034-triage-20180328 added

comment:13 Changed 9 months ago by nickm

Keywords: 034-removed-20180328 added

Per our triage process, these tickets are pending removal from 0.3.4.

comment:14 Changed 8 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: unspecified

These needs_revision, tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if somebody does the necessary revision.

Note: See TracTickets for help on using tickets.