From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH] net: fix problem in dequeuing from input_pkt_queue Date: Thu, 20 May 2010 06:37:26 +0200 Message-ID: <1274330246.2658.16.camel@edumazet-laptop> References: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Changli Gao , davem@davemloft.net, netdev@vger.kernel.org To: Tom Herbert Return-path: Received: from mail-ww0-f46.google.com ([74.125.82.46]:43229 "EHLO mail-ww0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751951Ab0ETEhd (ORCPT ); Thu, 20 May 2010 00:37:33 -0400 Received: by wwb18 with SMTP id 18so245449wwb.19 for ; Wed, 19 May 2010 21:37:31 -0700 (PDT) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Le mercredi 19 mai 2010 =C3=A0 19:48 -0700, Tom Herbert a =C3=A9crit : > >> It should be okay? process_backlog only runs in softirq so bottom > >> halves are already disabled, and I don't think flush_backlog runs = out > >> of an interrupt. > >> > > > > Oh no. It is an IRQ handler. > > > Very well, I will fix that. >=20 > Now I'm wondering, though, what the purpose of flush_backlog is... > since __netif_receive_skb is called with interrupts enabled it's > obvious flush_backlog won't catch all the skb's that reference the > device go away. Is there a reason these packets need to be flushed > and can't just be processed? flush_backlog is called when device is dismantled. No new packets should be generated by the device at this moment. Could you please split your patch in units, I spent 20 minutes to revie= w it and come to same conclusion than Changli (need to disable interrupts as they are currently disabled) and also : input_queue_head_incr(sd); are _not_ needed in flush_backlog() We are in the very last moments of the life of the device, in a very unlikely situation (packets in flight, not already consumed by the cpu)= , we are _dropping_ packets, so OOO means nothing at this point.=20 In dev_cpu_callback(), you reverse the order of input_pkt_queue / process_queue. Thats fine, but should be a single patch, because I am not sure the input_queue_head_incr() are valid here, since we re-inject these packet= s to netif_rx(). Could you clarify this point ? Thanks !