epoch support for taskqueues [2]
ClosedPublic
Actions

Authored by glebius on Feb 5 2020, 5:52 PM.

Details

Reviewers

gallatin
kib
bz
melifaro
manu
np
olivier
shurd
• hselasky
jeff

Group Reviewers

network
iflib

Commits

rS357772: Use NET_TASK_INIT() and NET_GROUPTASK_INIT() for drivers that process
rS357771: Add flag to struct task to mark the task as requiring network epoch.

Summary

[this is second invocation of D23408]

Add optional epoch to struct task and struct gtask. When processing a taskqueue and a task has associated epoch, then enter for duration of the task. If consecutive tasks belong to the same epoch, batch them.

Use NET_TASK_INIT() and NET_GROUPTASK_INIT() for drivers that process incoming packets in taskqueue context.

Add a temporary compatibility hack to ease transition to new ABI for people who have port modules (e.g. nvidia.ko).

https://github.com/glebius/FreeBSD/commits/taskq-epoch

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

glebius created this revision.Feb 5 2020, 5:52 PM

Herald added a reviewer: iflib. · View Herald TranscriptFeb 5 2020, 5:52 PM

Harbormaster completed remote builds in B29185: Diff 67820.Feb 5 2020, 5:52 PM

glebius retitled this revision from Summary: epoch support for taskqueues [2] to epoch support for taskqueues [2].Feb 5 2020, 5:52 PM

glebius edited the summary of this revision. (Show Details)

glebius mentioned this in D23408: epoch support for taskqueues.Feb 5 2020, 5:58 PM

• hselasky added inline comments.Feb 7 2020, 9:22 AM

sys/sys/taskqueue.h
113 ↗	(On Diff #67820)	Should you add: .ta_epoch = NULL Even if that is implicit?

glebius added inline comments.Feb 10 2020, 7:02 PM

sys/sys/taskqueue.h
113 ↗	(On Diff #67820)	I'd rather remove .ta_pending = 0 assignment, instead. The whole idea behind the C99 sparse initializers was to make it possible to change structure definition without necessity to touch every initializer. Explicit zeroing of every field brings functionality down to the level of C90 initializers.

I'm good with this change.

sys/sys/taskqueue.h
113 ↗	(On Diff #67820)	I'm fine with that.

Convert pointer in struct task to a flag.

Harbormaster completed remote builds in B29284: Diff 68101.Feb 10 2020, 11:56 PM

jeff accepted this revision.Feb 11 2020, 2:11 AM

jeff added inline comments.

sys/sys/_task.h
69–70 ↗	(On Diff #68101)	I would redefine these to be identical type widths to task. It should be ABI compatible with just more wasted space if I am reading it correctly.

• hselasky accepted this revision.Feb 11 2020, 8:31 AM

• hselasky added inline comments.

sys/sys/_task.h
52 ↗	(On Diff #68101)	In big endian machines this will be byte-swapped for binary compatibility (just a FYI) uint16_t ta_priority:8; uint16_t ta_flags:8; --HPS

• hselasky added inline comments.Feb 11 2020, 9:38 AM

sys/kern/subr_gtaskqueue.c
366 ↗	(On Diff #68101)	in_net_epoch needs update! It is currently only set once.
sys/kern/subr_taskqueue.c
469 ↗	(On Diff #68101)	Where are you updating in_net_epoch ?