Re: RFC: softint-based if_input

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Tue, Feb 2, 2016 at 10:07 AM, Taylor R Campbell
<campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
>    Date: Mon, 1 Feb 2016 19:18:44 +0900
>    From: Ryota Ozaki <ozaki-r%netbsd.org@localhost>
>
>    On Mon, Feb 1, 2016 at 2:41 PM, Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
>    > On Fri, Jan 29, 2016 at 11:58 PM, Taylor R Campbell
>    > <campbell+netbsd-tech-kern%mumble.net@localhost> 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