Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753602AbbH3QbI (ORCPT ); Sun, 30 Aug 2015 12:31:08 -0400 Received: from mail-wi0-f196.google.com ([209.85.212.196]:35589 "EHLO mail-wi0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751534AbbH3QbG (ORCPT ); Sun, 30 Aug 2015 12:31:06 -0400 MIME-Version: 1.0 In-Reply-To: <55E2CAE6.1040305@auriga.com> References: <55E2CAE6.1040305@auriga.com> Date: Sun, 30 Aug 2015 22:01:04 +0530 Message-ID: Subject: Re: [PATCH net-next 6/8] net: thunderx: Rework interrupt handler From: Sunil Kovvuri To: Aleksey Makarov Cc: Alexey Klimov , Linux Netdev List , Robert Richter , David Daney , Sunil Goutham , Aleksey Makarov , Linux Kernel Mailing List , Robert Richter , Sunil Goutham , LAKML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2246 Lines: 61 On Sun, Aug 30, 2015 at 2:50 PM, Aleksey Makarov wrote: > On 29.08.2015 04:44, Alexey Klimov wrote: > >>> -static irqreturn_t nicvf_intr_handler(int irq, void *nicvf_irq) >>> +static irqreturn_t nicvf_intr_handler(int irq, void *cq_irq) >>> +{ >>> + struct nicvf_cq_poll *cq_poll = (struct nicvf_cq_poll *)cq_irq; >>> + struct nicvf *nic = cq_poll->nicvf; >>> + int qidx = cq_poll->cq_idx; >>> + >>> + nicvf_dump_intr_status(nic); >>> + >>> + /* Disable interrupts */ >>> + nicvf_disable_intr(nic, NICVF_INTR_CQ, qidx); >>> + >>> + /* Schedule NAPI */ >>> + napi_schedule(&cq_poll->napi); >>> + >>> + /* Clear interrupt */ >>> + nicvf_clear_intr(nic, NICVF_INTR_CQ, qidx); >>> + >>> + return IRQ_HANDLED; >>> +} >> >> >> You're not considering spurious irqs in all new irq handlers here and >> below and schedule napi/tasklets unconditionally. Is it correct? >> For me it looks like previous implementation relied on reading of >> NIC_VF_INT to understand irq type and what actions should be >> performed. It generally had idea that no interrupt might occur. > > > 1. The previous version of the handler did not handle spurious interrupts > either. Probably that means that the author of the patch knows for sure > that they do not happen. Yes, no spurious interrupts are expected from hardware. Even if it does the NAPI poll routine will handle it as valid descriptor count would be zero. Don't think it makes sense to check for spurious interrupt upon every interrupt. > > 2. Instead of reading the status register new version registers different > handlers for different irqs. I don't see why it can be wrong. Previous implementation results on scheduling multiple NAPI poll handlers on the same CPU even if IRQ's affinities are set to different CPUs. Hence they are seperated now. > > I am going to address your other suggestions in the next version of the > patchset. > > Thank you > Aleksey Makarov > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/