Changeset View
Standalone View
sys/netinet/tcp_hpts.c
Show First 20 Lines • Show All 187 Lines • ▼ Show 20 Lines | struct tcp_hpts_entry { | ||||
uint32_t p_cur_slot; /* Current slot in wheel hpts is draining */ | uint32_t p_cur_slot; /* Current slot in wheel hpts is draining */ | ||||
uint32_t p_nxt_slot; /* The next slot outside the current range of | uint32_t p_nxt_slot; /* The next slot outside the current range of | ||||
* slots that the hpts is running on. */ | * slots that the hpts is running on. */ | ||||
int32_t p_on_queue_cnt; /* Count on queue in this hpts */ | int32_t p_on_queue_cnt; /* Count on queue in this hpts */ | ||||
uint32_t p_lasttick; /* Last tick before the current one */ | uint32_t p_lasttick; /* Last tick before the current one */ | ||||
uint8_t p_direct_wake :1, /* boolean */ | uint8_t p_direct_wake :1, /* boolean */ | ||||
p_on_min_sleep:1, /* boolean */ | p_on_min_sleep:1, /* boolean */ | ||||
p_hpts_wake_scheduled:1, /* boolean */ | p_hpts_wake_scheduled:1, /* boolean */ | ||||
p_avail:5; | hit_callout_thresh:1, | ||||
p_avail:4; | |||||
uint8_t p_fill[3]; /* Fill to 32 bits */ | uint8_t p_fill[3]; /* Fill to 32 bits */ | ||||
/* Cache line 0x40 */ | /* Cache line 0x40 */ | ||||
struct hptsh { | struct hptsh { | ||||
TAILQ_HEAD(, tcpcb) head; | TAILQ_HEAD(, tcpcb) head; | ||||
uint32_t count; | uint32_t count; | ||||
uint32_t gencnt; | uint32_t gencnt; | ||||
} *p_hptss; /* Hptsi wheel */ | } *p_hptss; /* Hptsi wheel */ | ||||
uint32_t p_hpts_sleep_time; /* Current sleep interval having a max | uint32_t p_hpts_sleep_time; /* Current sleep interval having a max | ||||
* of 255ms */ | * of 255ms */ | ||||
uint32_t overidden_sleep; /* what was overrided by min-sleep for logging */ | uint32_t overidden_sleep; /* what was overrided by min-sleep for logging */ | ||||
uint32_t saved_lasttick; /* for logging */ | uint32_t saved_lasttick; /* for logging */ | ||||
uint32_t saved_curtick; /* for logging */ | uint32_t saved_curtick; /* for logging */ | ||||
uint32_t saved_curslot; /* for logging */ | uint32_t saved_curslot; /* for logging */ | ||||
uint32_t saved_prev_slot; /* for logging */ | uint32_t saved_prev_slot; /* for logging */ | ||||
uint32_t p_delayed_by; /* How much were we delayed by */ | uint32_t p_delayed_by; /* How much were we delayed by */ | ||||
/* Cache line 0x80 */ | /* Cache line 0x80 */ | ||||
struct sysctl_ctx_list hpts_ctx; | struct sysctl_ctx_list hpts_ctx; | ||||
struct sysctl_oid *hpts_root; | struct sysctl_oid *hpts_root; | ||||
struct intr_event *ie; | struct intr_event *ie; | ||||
void *ie_cookie; | void *ie_cookie; | ||||
uint16_t p_num; /* The hpts number one per cpu */ | uint16_t p_num; /* The hpts number one per cpu */ | ||||
uint16_t p_cpu; /* The hpts CPU */ | uint16_t p_cpu; /* The hpts CPU */ | ||||
/* There is extra space in here */ | /* There is extra space in here */ | ||||
tuexen: Would it make sense to use for this one bit in `p_avail` from line 196, since you use this a… | |||||
Done Inline ActionsYeah I had forgotten that I had a bit array already rrs: Yeah I had forgotten that I had a bit array already | |||||
/* Cache line 0x100 */ | /* Cache line 0x100 */ | ||||
struct callout co __aligned(CACHE_LINE_SIZE); | struct callout co __aligned(CACHE_LINE_SIZE); | ||||
} __aligned(CACHE_LINE_SIZE); | } __aligned(CACHE_LINE_SIZE); | ||||
static struct tcp_hptsi { | static struct tcp_hptsi { | ||||
struct cpu_group **grps; | struct cpu_group **grps; | ||||
struct tcp_hpts_entry **rp_ent; /* Array of hptss */ | struct tcp_hpts_entry **rp_ent; /* Array of hptss */ | ||||
uint32_t *cts_last_ran; | uint32_t *cts_last_ran; | ||||
Show All 36 Lines | |||||
static int32_t tcp_hpts_precision = 120; | static int32_t tcp_hpts_precision = 120; | ||||
static struct hpts_domain_info { | static struct hpts_domain_info { | ||||
int count; | int count; | ||||
int cpu[MAXCPU]; | int cpu[MAXCPU]; | ||||
} hpts_domains[MAXMEMDOM]; | } hpts_domains[MAXMEMDOM]; | ||||
counter_u64_t hpts_hopelessly_behind; | counter_u64_t hpts_hopelessly_behind; | ||||
Not Done Inline ActionsCan you declare this in subr_trap.c, so its on the same cacheline as the tcp_hpts_softclock function pointer? gallatin: Can you declare this in subr_trap.c, so its on the same cacheline as the tcp_hpts_softclock… | |||||
Done Inline ActionsSure that makes sense. It will require not being static but thats probably ok :) rrs: Sure that makes sense. It will require not being static but thats probably ok :) | |||||
SYSCTL_COUNTER_U64(_net_inet_tcp_hpts_stats, OID_AUTO, hopeless, CTLFLAG_RD, | SYSCTL_COUNTER_U64(_net_inet_tcp_hpts_stats, OID_AUTO, hopeless, CTLFLAG_RD, | ||||
&hpts_hopelessly_behind, | &hpts_hopelessly_behind, | ||||
"Number of times hpts could not catch up and was behind hopelessly"); | "Number of times hpts could not catch up and was behind hopelessly"); | ||||
counter_u64_t hpts_loops; | counter_u64_t hpts_loops; | ||||
SYSCTL_COUNTER_U64(_net_inet_tcp_hpts_stats, OID_AUTO, loops, CTLFLAG_RD, | SYSCTL_COUNTER_U64(_net_inet_tcp_hpts_stats, OID_AUTO, loops, CTLFLAG_RD, | ||||
▲ Show 20 Lines • Show All 48 Lines • ▼ Show 20 Lines | SYSCTL_INT(_net_inet_tcp_hpts, OID_AUTO, bind_hptss, CTLFLAG_RD, | ||||
"Thread Binding tunable"); | "Thread Binding tunable"); | ||||
SYSCTL_INT(_net_inet_tcp_hpts, OID_AUTO, use_irq, CTLFLAG_RD, | SYSCTL_INT(_net_inet_tcp_hpts, OID_AUTO, use_irq, CTLFLAG_RD, | ||||
&tcp_use_irq_cpu, 0, | &tcp_use_irq_cpu, 0, | ||||
"Use of irq CPU tunable"); | "Use of irq CPU tunable"); | ||||
SYSCTL_INT(_net_inet_tcp_hpts, OID_AUTO, precision, CTLFLAG_RW, | SYSCTL_INT(_net_inet_tcp_hpts, OID_AUTO, precision, CTLFLAG_RW, | ||||
&tcp_hpts_precision, 120, | &tcp_hpts_precision, 120, | ||||
"Value for PRE() precision of callout"); | "Value for PRE() precision of callout"); | ||||
SYSCTL_INT(_net_inet_tcp_hpts, OID_AUTO, cnt_thresh, CTLFLAG_RW, | SYSCTL_INT(_net_inet_tcp_hpts, OID_AUTO, cnt_thresh, CTLFLAG_RW, | ||||
&conn_cnt_thresh, 0, | &conn_cnt_thresh, 0, | ||||
Not Done Inline ActionsI think 0 is correct here. Since the pointer is not NULL, the parameter after pointer is basically ignored. tuexen: I think 0 is correct here. Since the pointer is not NULL, the parameter after pointer is… | |||||
Done Inline ActionsHmm is not the next parameter the "default value"? conn_cnt_thresh = DEFAULT_CONNECTION_THRESHOLD; at line 245, so it does not default to 0, but 100. rrs: Hmm is not the next parameter the "default value"?
And if so its set:
conn_cnt_thresh =… | |||||
Not Done Inline ActionsYes, the default comes from line 245. man SYSCTL_INT says: struct sysctl_oid * SYSCTL_ADD_INT(struct sysctl_ctx_list *ctx, struct sysctl_oid_list *parent, int number, const char *name, int ctlflags, int *ptr, int val, const char *descr); ... ptr Pointer to sysctl variable or string data. For sysctl values the pointer can be SYSCTL_NULL_XXX_PTR which means the OID is read-only and the returned value should be taken from the val argument. val If the ptr argument is SYSCTL_NULL_XXX_PTR, gives the constant value returned by this OID. Else this argument is not used. tuexen: Yes, the default comes from line 245.
`man SYSCTL_INT` says:
```
struct sysctl_oid *… | |||||
"How many connections (below) make us use the callout based mechanism"); | "How many connections (below) make us use the callout based mechanism"); | ||||
SYSCTL_INT(_net_inet_tcp_hpts, OID_AUTO, logging, CTLFLAG_RW, | SYSCTL_INT(_net_inet_tcp_hpts, OID_AUTO, logging, CTLFLAG_RW, | ||||
&hpts_does_tp_logging, 0, | &hpts_does_tp_logging, 0, | ||||
"Do we add to any tp that has logging on pacer logs"); | "Do we add to any tp that has logging on pacer logs"); | ||||
SYSCTL_INT(_net_inet_tcp_hpts, OID_AUTO, dyn_minsleep, CTLFLAG_RW, | SYSCTL_INT(_net_inet_tcp_hpts, OID_AUTO, dyn_minsleep, CTLFLAG_RW, | ||||
&dynamic_min_sleep, 250, | &dynamic_min_sleep, 250, | ||||
"What is the dynamic minsleep value?"); | "What is the dynamic minsleep value?"); | ||||
SYSCTL_INT(_net_inet_tcp_hpts, OID_AUTO, dyn_maxsleep, CTLFLAG_RW, | SYSCTL_INT(_net_inet_tcp_hpts, OID_AUTO, dyn_maxsleep, CTLFLAG_RW, | ||||
▲ Show 20 Lines • Show All 1,197 Lines • ▼ Show 20 Lines | |||||
static void | static void | ||||
__tcp_run_hpts(void) | __tcp_run_hpts(void) | ||||
{ | { | ||||
struct epoch_tracker et; | struct epoch_tracker et; | ||||
struct tcp_hpts_entry *hpts; | struct tcp_hpts_entry *hpts; | ||||
int ticks_ran; | int ticks_ran; | ||||
hpts = tcp_choose_hpts_to_run(); | hpts = tcp_choose_hpts_to_run(); | ||||
Not Done Inline ActionsCan you move this test into the #define of tcp_hpts_softclock in systm.h ? That would avoid a function call. gallatin: Can you move this test into the #define of tcp_hpts_softclock in systm.h ? That would avoid a… | |||||
Done Inline ActionsSure rrs: Sure | |||||
if (hpts->p_hpts_active) { | if (hpts->p_hpts_active) { | ||||
/* Already active */ | /* Already active */ | ||||
return; | return; | ||||
} | } | ||||
if (mtx_trylock(&hpts->p_mtx) == 0) { | if (mtx_trylock(&hpts->p_mtx) == 0) { | ||||
/* Someone else got the lock */ | /* Someone else got the lock */ | ||||
return; | return; | ||||
▲ Show 20 Lines • Show All 118 Lines • ▼ Show 20 Lines | if (hpts->p_hpts_active) { | ||||
} | } | ||||
goto back_to_sleep; | goto back_to_sleep; | ||||
} | } | ||||
hpts->sleeping = 0; | hpts->sleeping = 0; | ||||
hpts->p_hpts_active = 1; | hpts->p_hpts_active = 1; | ||||
ticks_ran = tcp_hptsi(hpts, 1); | ticks_ran = tcp_hptsi(hpts, 1); | ||||
tv.tv_sec = 0; | tv.tv_sec = 0; | ||||
tv.tv_usec = hpts->p_hpts_sleep_time * HPTS_TICKS_PER_SLOT; | tv.tv_usec = hpts->p_hpts_sleep_time * HPTS_TICKS_PER_SLOT; | ||||
if ((hpts->p_on_queue_cnt > conn_cnt_thresh) && (hpts->hit_callout_thresh == 0)) { | |||||
hpts->hit_callout_thresh = 1; | |||||
atomic_add_int(&hpts_that_need_softclock, 1); | |||||
Not Done Inline ActionsWould it make sense to pul the above code after line 516, where p_on_queue_cnt is incremented? tuexen: Would it make sense to pul the above code after line 516, where `p_on_queue_cnt` is incremented? | |||||
Done Inline ActionsI wanted the place where we were exiting, and since we already are making comparisons At the insert-internal point you are not through processing all the inp's that are "to be processed". I thought it best rrs: I wanted the place where we were exiting, and since we already are making comparisons
to this… | |||||
} else if ((hpts->p_on_queue_cnt <= conn_cnt_thresh) && (hpts->hit_callout_thresh == 1)) { | |||||
hpts->hit_callout_thresh = 0; | |||||
atomic_subtract_int(&hpts_that_need_softclock, 1); | |||||
} | |||||
Not Done Inline ActionsWould it make sense to put the above lines after line 585, where p_on_queue_cnt is decremented? tuexen: Would it make sense to put the above lines after line 585, where `p_on_queue_cnt` is… | |||||
if (hpts->p_on_queue_cnt >= conn_cnt_thresh) { | if (hpts->p_on_queue_cnt >= conn_cnt_thresh) { | ||||
if(hpts->p_direct_wake == 0) { | if(hpts->p_direct_wake == 0) { | ||||
/* | /* | ||||
* Only adjust sleep time if we were | * Only adjust sleep time if we were | ||||
* called from the callout i.e. direct_wake == 0. | * called from the callout i.e. direct_wake == 0. | ||||
*/ | */ | ||||
if (ticks_ran < ticks_indicate_more_sleep) { | if (ticks_ran < ticks_indicate_more_sleep) { | ||||
hpts->p_mysleep.tv_usec *= 2; | hpts->p_mysleep.tv_usec *= 2; | ||||
▲ Show 20 Lines • Show All 396 Lines • Show Last 20 Lines |
Would it make sense to use for this one bit in p_avail from line 196, since you use this a boolean?