Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754267AbcDKSDY (ORCPT ); Mon, 11 Apr 2016 14:03:24 -0400 Received: from mail-yw0-f172.google.com ([209.85.161.172]:36821 "EHLO mail-yw0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752890AbcDKSDU (ORCPT ); Mon, 11 Apr 2016 14:03:20 -0400 MIME-Version: 1.0 In-Reply-To: <1460389951.6473.555.camel@edumazet-glaptop3.roam.corp.google.com> References: <1460355869-13539-1-git-send-email-larper@axis.com> <1460380981.6473.544.camel@edumazet-glaptop3.roam.corp.google.com> <570BA8C7.1000905@axis.com> <1460384551.6473.551.camel@edumazet-glaptop3.roam.corp.google.com> <570BC024.1070504@axis.com> <1460389951.6473.555.camel@edumazet-glaptop3.roam.corp.google.com> From: Cong Wang Date: Mon, 11 Apr 2016 11:02:59 -0700 Message-ID: Subject: Re: [PATCH net v2] net: sched: do not requeue a NULL skb To: Eric Dumazet Cc: Lars Persson , Lars Persson , Linux Kernel Network Developers , Jamal Hadi Salim , LKML 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: 1317 Lines: 35 On Mon, Apr 11, 2016 at 8:52 AM, Eric Dumazet wrote: > On Mon, 2016-04-11 at 17:17 +0200, Lars Persson wrote: >> >> On 04/11/2016 04:22 PM, Eric Dumazet wrote: >> > On Mon, 2016-04-11 at 15:38 +0200, Lars Persson wrote: >> > >> >> I though it would be prudent because the queue can be non-empty even for >> >> the case of skb=NULL. So should it be there in this patch, another patch >> >> or not at all ? >> > >> > Then maybe change return code ? >> > >> > It seems strange that a validate_xmit_skb_list() failure stops the >> > __qdisc_run() loop but schedules another round. >> > >> > >> >> It was suggested by Cong Wang to return 0 in order to stop the loop. Do >> you guys agree that the loop should be stopped for such failures ? Then >> I will put the schedule call inside the if as you proposed earlier. > > What are the causes of validate_xmit_skb_list() failures ? > > If gso segmentations fail because of memory pressure, better free more > skbs right now. > > In any case, having a single test " if (skb) " sounds better to me, > to have a fast path. > > So your first patch was probably a better idea. > > v2 has two tests instead of one. I am fine with either way as long as the loop stops on failure. Folding the test "if (skb)" into one also requires to retake the spinlock.