On Mon, Apr 11, 2016 at 8:52 AM, Eric Dumazet <eric.duma...@gmail.com> 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.
