From mboxrd@z Thu Jan  1 00:00:00 1970
From: John Fastabend <john.fastabend@gmail.com>
Subject: Re: [RFC PATCH 06/17] net: sched: explicit locking in gso_cpu
 fallback
Date: Wed, 15 Nov 2017 07:11:58 -0800
Message-ID: <09820d68-ea84-5c9a-bb16-bc99f268c253@gmail.com>
References: <20171113195256.6245.64676.stgit@john-Precision-Tower-5810>
 <20171113200935.6245.70791.stgit@john-Precision-Tower-5810>
 <CAF=yD-+RS1v5vYy5ynQLaioc-527YN4a-P_+fvJ5AxeWLdqmwA@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: Daniel Borkmann <daniel@iogearbox.net>,
        Eric Dumazet <eric.dumazet@gmail.com>, make0818@gmail.com,
        Network Development <netdev@vger.kernel.org>,
        =?UTF-8?B?SmnFmcOtIFDDrXJr?= =?UTF-8?Q?o?= <jiri@resnulli.us>,
        Cong Wang <xiyou.wangcong@gmail.com>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pf0-f193.google.com ([209.85.192.193]:55601 "EHLO
        mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1755744AbdKOPMX (ORCPT
        <rfc822;netdev@vger.kernel.org>); Wed, 15 Nov 2017 10:12:23 -0500
Received: by mail-pf0-f193.google.com with SMTP id 17so17210939pfn.12
        for <netdev@vger.kernel.org>; Wed, 15 Nov 2017 07:12:23 -0800 (PST)
In-Reply-To: <CAF=yD-+RS1v5vYy5ynQLaioc-527YN4a-P_+fvJ5AxeWLdqmwA@mail.gmail.com>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On 11/14/2017 04:41 PM, Willem de Bruijn wrote:
>>  /* use instead of qdisc->dequeue() for all qdiscs queried with ->peek() */
>>  static inline struct sk_buff *qdisc_dequeue_peeked(struct Qdisc *sch)
>>  {
>> -       struct sk_buff *skb = sch->gso_skb;
>> +       struct sk_buff *skb = skb_peek(&sch->gso_skb);
>>
>>         if (skb) {
>> -               sch->gso_skb = NULL;
>> +               skb = __skb_dequeue(&sch->gso_skb);
>>                 qdisc_qstats_backlog_dec(sch, skb);
>>                 sch->q.qlen--;
> 
> In lockless qdiscs, can this race, so that __skb_dequeue returns NULL?
> Same for its use in qdisc_peek_dequeued.
> 

Yes, agree if this was used in lockless qdisc it could race. However,
I don't think it is actually used in the lockless cases yet. For pfifo
fast __skb_array_peek is used.

>> -static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
>> +static inline int __dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
>>  {
> 
> Perhaps dev_requeue_skb_qdisc_locked is more descriptive. Or
> adding a lockdep_is_held(..) also documents that the __locked variant
> below is not just a lock/unlock wrapper around this inner function.
> 

Adding lockdep annotation here makes sense to me.