On 06/12/15(Sun) 14:00, David Gwynne wrote: > the current code for serialising if_start calls for mpsafe nics does > what it says. > > however, kettenis realised it doesnt help us much when we're trying > to coordinate between the start and txeof side of a driver when > setting or clearing oactive. in particular, a start routine can > figure out there's no more space, and then set oactive. txeof could > be running on another cpu emptying the ring and clearing it. if > that clear runs in between the other cpus space check and > ifq_set_oactive, then the nic will be marked full and the stack > wont ever call start on it again. > > so it can be argued that start and txeof should be serialised. > indeed, other platforms do exactly that. > > the least worst mechanism we have for doing that is taskqs. however, > all my experiments deferring start to a taskq end up significantly > hurting performance. > [...] > while toying with ideas on how to solve kettenis' oactive problem, > i came up with the following. > > it combines tasks with direct dispatch, and borrows the current > ifq_serialiser/pool/scsi serialisation algorithm.
I like the idea. > the idea is you have a taskctx, which represents a serialising > context for tasks. tasks are submitted to the taskctx, and the code > will try to run the tasks immediately rather than defer them to a > thread. if there is contention on the context, the contending cpu > yields after queueing the task because the other cpu is responsible > for running all pending tasks to completion. > > it also simplifies the barrier operations a lot. > > the diff below implements a generic taskctx framework, and cuts the > mpsafe if_start() implementation over to it. I'm not sure this should be implemented as a generic framework. I'd suggest to keep it specific to if_start(). As you say above the least worst mechanism we have is currently taskqs, but maybe this could/will change? I cannot understand what's happening by reading the myx(4) chunk itself. So I believe the interface needs to be polished. Would it be possible to implement this serialization without introducing if_restart()? > myx is also changed to only clr oactive from within the taskctx > serialiser, thereby avoiding the race, but keeps the bulk of txeof > outside the serialiser so it can run concurrently with start. > > other nics are free to serialise start and txeof within the > ifq_serializer if they want, or not, it is up to them. > > thoughts? tests? opinions on messy .h files? It appears to me that you have unrelated changes: if_enter/if_leave.
