[PATCH 1/2] staging: octeon-ethernet: don't assume that CPU 0 is special
David Daney
ddaney at caviumnetworks.com
Mon Sep 30 17:23:10 UTC 2013
On 09/28/2013 12:50 PM, Aaro Koskinen wrote:
> Currently the driver assumes that CPU 0 is handling all the hard IRQs.
> This is wrong in Linux SMP systems where user is allowed to assign to
> hardware IRQs to any CPU. The driver will stop working if user sets
> smp_affinity so that interrupts end up being handled by other than CPU
> 0. The patch fixes that.
>
> Signed-off-by: Aaro Koskinen <aaro.koskinen at iki.fi>
> ---
> drivers/staging/octeon/ethernet-rx.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
> index e14a1bb..de831c1 100644
> --- a/drivers/staging/octeon/ethernet-rx.c
> +++ b/drivers/staging/octeon/ethernet-rx.c
> @@ -80,6 +80,8 @@ struct cvm_oct_core_state {
>
> static struct cvm_oct_core_state core_state __cacheline_aligned_in_smp;
>
> +static int cvm_irq_cpu = -1;
> +
> static void cvm_oct_enable_napi(void *_)
> {
> int cpu = smp_processor_id();
> @@ -112,11 +114,7 @@ static void cvm_oct_no_more_work(void)
> {
> int cpu = smp_processor_id();
>
> - /*
> - * CPU zero is special. It always has the irq enabled when
> - * waiting for incoming packets.
> - */
> - if (cpu == 0) {
> + if (cpu == cvm_irq_cpu) {
> enable_irq(OCTEON_IRQ_WORKQ0 + pow_receive_group);
> return;
> }
> @@ -135,6 +133,7 @@ static irqreturn_t cvm_oct_do_interrupt(int cpl, void *dev_id)
> {
> /* Disable the IRQ and start napi_poll. */
> disable_irq_nosync(OCTEON_IRQ_WORKQ0 + pow_receive_group);
> + cvm_irq_cpu = smp_processor_id();
> cvm_oct_enable_napi(NULL);
>
> return IRQ_HANDLED;
> @@ -547,8 +546,9 @@ void cvm_oct_rx_initialize(void)
> cvmx_write_csr(CVMX_POW_WQ_INT_PC, int_pc.u64);
>
>
> - /* Scheduld NAPI now. This will indirectly enable interrupts. */
> + /* Schedule NAPI now. */
> cvm_oct_enable_one_cpu();
> + enable_irq(OCTEON_IRQ_WORKQ0 + pow_receive_group);
The fact that you have to manually enable irqs here indicates that the
patch is not good.
Either the enable_irq() is unnecessary, or you broke the logic for
enabling NAPI on more than one CPU.
I am not sure which is the case, but I think it would be best if you
supplied a fixed patch set that corrects whichever happens to be the case.
David Daney
> }
>
> void cvm_oct_rx_shutdown(void)
>
More information about the devel
mailing list