On Tue, Feb 2, 2016 at 10:07 AM, Taylor R Campbell <campbell+netbsd-tech-k...@mumble.net> wrote: > Date: Mon, 1 Feb 2016 19:18:44 +0900 > From: Ryota Ozaki <ozak...@netbsd.org> > > On Mon, Feb 1, 2016 at 2:41 PM, Ryota Ozaki <ozak...@netbsd.org> wrote: > > On Fri, Jan 29, 2016 at 11:58 PM, Taylor R Campbell > > <campbell+netbsd-tech-k...@mumble.net> wrote: > > So I think it's better to adopt two approaches: > > - Add if_percpuq to ifnet and let drivers use it by default (via > if_attach) > > - Provide a new API that you propose, and let drivers that are ready > > for softint use it (we use if_initialize instead) > > - Change drivers to use the new API one by one (it would take long time) > > - Once we migrate all drivers, then remove if_percpuq from ifnet > > (I'm assuming if_percpuq is in ifdef _KERNEL and removable) > > > > Does this approach satisfy you (or not)? > > http://www.netbsd.org/~ozaki-r/softint-if_input-percpuq.diff > > This is a patch that uses if_percpuq. There is no actual > user of it yet. I'm trying to apply it to wm. > > That looks more or less fine. A couple correctness notes: > > - if_percpu_enqueue: I think you should do splnet, in case some driver > might call it at different IPLs -- like bcmeth(4) which may do it from > thread context or from softint context.
I guess if a driver needs splnet, the driver already has it, but anyway I added just in case :) (And I think splnet should be in if_percpu_enqueue (common place), not in each driver, so we may be able to get of driver's one.) > > - if_percpuq_purge_ifq: You may need to disestablish the softint > before touching remote CPUs' queues; otherwise softints might still be > in flight. Sure. Fixed. > > I would have put creation of the if_percpuq in a separate routine so > that you don't have to change callers whose behaviour is not changing; > rather you only have to change callers that positively want to use > if_percpuq. I might add another field to if_percpuq which is an extra > argument to pass, so that the callback function can take both the ifq > and an extra argument, as I described in my last message. I added ifp to struct if_percpuq for ifp->_if_input to pass both ifp and ipq via softint_estabalish. Here is a new patch: http://www.netbsd.org/~ozaki-r/softint-if_input-percpuq2.diff The diff from v1 is here: http://www.netbsd.org/~ozaki-r/softint-if_input-percpuq2-diff.diff Summary of the changes: - if_percpuq_create doesn't take callback - if_percpuq has struct ifnet * - for ifp->_if_input in softint - softint_establish takes if_percpuq isntead of ifnet - if_percpuq_dequeue is implemented for API consistency - may not need - if_percpuq_enqueue has splnet - KASSERT(cpu_intr_p()) is removed from if_input - wm uses if_percpuq directly - as a reference implementation How about it? Thanks, ozaki-r
