Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Message-ID: <f3e84a37-3218-0d52-e7ed-2d215fed58e3@intel.com>
Date:   Tue, 12 Sep 2023 17:57:26 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH] net/core: Export dev_core_stats_rx_dropped_inc sets
Content-Language: en-US
To:     Eric Dumazet <edumazet@google.com>
CC:     Yajun Deng <yajun.deng@linux.dev>, <davem@davemloft.net>,
        <kuba@kernel.org>, <pabeni@redhat.com>, <horms@kernel.org>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20230911082016.3694700-1-yajun.deng@linux.dev>
 <CANn89i+W1iAQmOhunLbqpvHu8EUO6uawv6Uvx7qimyBa_PBNCg@mail.gmail.com>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <CANn89i+W1iAQmOhunLbqpvHu8EUO6uawv6Uvx7qimyBa_PBNCg@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WnJkSVdTZGRoZnpvRFNPRFJLRFgrRnlVK3pwYlNteWxacFNHcWs3dVV3Rm9C?=
 =?utf-8?B?ZWNON1liT2ZFKzVYOS96YUNtTXBKZUxMb2kwaGwyeGhKZEU5ZTRBYUtlRks5?=
 =?utf-8?B?bW0rRUgvS1VQMnFDMlNqb2RnTmZxQ3dsckpFRm9EQVpkNy9aQVFQN3lKUEo1?=
 =?utf-8?B?MjM2ZXR6WkVOY1M2bHRMQi9EYngraVg4K1Vta1YzUTFuWk95ZFdPZVJ1RTFS?=
 =?utf-8?B?WUtsSVBlVldsSWV0WUdtYmxUcFlmbUowUVJCclhySlNIN3JVb3RnNHh3ajI5?=
 =?utf-8?B?dWw2d0Zrc2dMK21zN1JmMVlRemU5SkF6dDliT2hOTlJkdFN4b3h1ZjBCZ3ZB?=
 =?utf-8?B?Q2ZYdDhoNDRybmNwZzRyOTVnakEvLzNpQlFTblFvblRUdlhoV21pK0hrSnhG?=
 =?utf-8?B?dVlCYzhEWHErUkN4VTJicC9iL20yZlpQWDY1VnlEdnBYNHFYR0Rxd0cxQlNY?=
 =?utf-8?B?cHdXVXoxUTRYeFBETFcycWYraXlNMy9Ua1phMlFKRnM2MlhvNUtPME9KbUk0?=
 =?utf-8?B?dXQydjRVdnNSQWxZREVobDFnV1VFUEtNZ0JBWVZ1NVJoWWFkNUZwaVAzRHRR?=
 =?utf-8?B?bFJhcjBPMmp5YkpFUG5JSzZUQ0RMOVBRNGRCTWZwOU5NbzR2TEFFUnpIYlpF?=
 =?utf-8?B?bWhlT0JMVzFscFQwbUlaS0NoNForaFZsb2krYlFsaFhQeGoreHpXa0pCMFdZ?=
 =?utf-8?B?NHVPeGs1NHJ5ZlA1MTVmMlNTTVFDcXArNGs5ZHVpYk4veDRxZWJWNkNnTW55?=
 =?utf-8?B?dWpRbjJIeHVqK1BiRzJkcndXdlJLNklRT2E4TXlxVXlzekkvdkxLQmxQUVRN?=
 =?utf-8?B?UnRXaFdVZEFmWW5MZVBDMlRLeURQd1hEQURBSUZqQU53V01EMG1HbndUMFk2?=
 =?utf-8?B?UjhUZUlmdGdKU0dSTGxWTzJ2a2JIR0pMTzlHcktRWlM5YXZmQStUck1naUx1?=
 =?utf-8?B?My9nYmk4eXFoVXpVVDVKMUZleHBaTWl4QXFzVkM3cENMUVBQWTl4NW96b203?=
 =?utf-8?B?R2twSHo5TUc3bk1kVlpSS1A0Q1V5emFZZGdDVWt6ZU5kKzl3TWowSFJVYjRC?=
 =?utf-8?B?TGpFUTArcUtvWlNsd1p3MUMrU291MzM2Rzc5ME5odktER1QrNWtDUGE0Um5s?=
 =?utf-8?B?Q00zL3didXpLOW9OcUNkRm94SWJZRi92UHJpSmwvekQrbXJPRVN4aC9kK0tX?=
 =?utf-8?B?Nm1pTGgyN2JaOFJta3ZNYUlxYVVsNEl6YkhzeGJNODhZdlFBWEdhQWp4Q3Fi?=
 =?utf-8?B?SnY0d0ZEWVlLTE1Zc0dVZXk4Z2hZVDdSTEVrZTRTMFNOOHAzWFNxY1JoNlNW?=
 =?utf-8?B?YUZFMzZnUFFZb3FyMGQzNVlsSm8zNVJ3MG52S2lCeXhjd21zOVVRS2VEd21m?=
 =?utf-8?B?MndwcC9RRnJaTHowRERWY2FXY2VWcWY4ZjBXK2hGcW0xclNOUEo5cER6aGVl?=
 =?utf-8?B?K21yTklDSHRIcEVjd2drN0lBbHM0dHdKWGZmeDNOcTFxc0VOUjJBTE53Yngy?=
 =?utf-8?B?bGw3MHdkWXcwdmRjaVFwK29oRTAzdkJ4QWJ1WnV4ZERqZG1yemMwdFpPNlpp?=
 =?utf-8?B?b0w2VGQzMXFRMlN6MWhlZ3ljS2F0Mi9ldDhGRmF2VG9IVzBxZUVvWkhPRi9n?=
 =?utf-8?B?MWorTnRzY1plN0Z3ZlNiMkJkdTFVeE45ZVJhVDc0WmY4Nk1samhzL01qcVIw?=
 =?utf-8?B?R2pScjlTMytVQmd4QlV3UFZNWUxPa0J6dU9xWnpjRklYWlJFQ1N3eEI0YnFI?=
 =?utf-8?B?dDgrOGhSZ0poOStUT3BlUnFIRzR4Q0QxQVEzTHU0b2ltQjFDYnEzTGpBUDlj?=
 =?utf-8?B?Q0wrVU5odG9sczF4ZHhXRlNXYUtDTE9jekNYUE94VGhtOGh3Wm5wVmFDdFk1?=
 =?utf-8?B?QzhkMkpld1NOYldlU0h1NFZCbmgwQ3RMU0s1M1pWTWh6cGNsRWhyUFErL0FH?=
 =?utf-8?B?V1QyWk5TQnpCQUtERkd5VHFkOER4NzJEY0tGd0ltWTE1SUFuV1BOWmkza1E0?=
 =?utf-8?B?MXl6N1pLemhvNHNNSDh1VmR3UFV0SmtqQW1OZFo4dlVVZE95ZjdNVjBaTDdn?=
 =?utf-8?B?S0RWUndhcXZKek1EUmlRcHE5L3ZQd3ZwekNnOTVZdFYzT0FjdjhtcWJZY0RO?=
 =?utf-8?B?QUlzTmsyb2YrNUI5Nm5NSThuT290cWdyK3V4dnpHK2tXdHBlQTQ2TXpqYmgz?=
 =?utf-8?B?M2c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3f078341-ff88-4041-7f24-08dbb3a90abf
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Sep 2023 15:58:01.6345
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: VAP2UrvRTgCBcvJasARaDi+s9JnStNwpbkc9E8PJBbw1jy5lMdK9PnoUnhLFAGh/9yq+Q5NXvvrur8YdxRNX32mLUydcCZ6S7HA+gIgEPKQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB6618
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Tue, 12 Sep 2023 08:58:18 -0700 (PDT)
X-Spam-Status: No, score=-2.3 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email

From: Eric Dumazet <edumazet@google.com>
Date: Tue, 12 Sep 2023 06:23:24 +0200

> On Mon, Sep 11, 2023 at 10:20 AM Yajun Deng <yajun.deng@linux.dev> wrote:
>>
>> Although there is a kfree_skb_reason() helper function that can be used
>> to find the reason for dropped packets, but most callers didn't increase
>> one of rx_dropped, tx_dropped, rx_nohandler and rx_otherhost_dropped.

[...]

>>  EXPORT_SYMBOL(netdev_stats_to_stats64);
>>
>> -struct net_device_core_stats __percpu *netdev_core_stats_alloc(struct net_device *dev)
>> +static struct net_device_core_stats __percpu *netdev_core_stats_alloc(struct net_device *dev)
>>  {
>>         struct net_device_core_stats __percpu *p;
>>
>> @@ -10488,7 +10488,33 @@ struct net_device_core_stats __percpu *netdev_core_stats_alloc(struct net_device
>>         /* This READ_ONCE() pairs with the cmpxchg() above */
>>         return READ_ONCE(dev->core_stats);
>>  }
>> -EXPORT_SYMBOL(netdev_core_stats_alloc);
>> +
>> +static inline struct net_device_core_stats __percpu *dev_core_stats(struct net_device *dev)
> 
> Please remove this inline attritbute. Consider using __cold instead.

__cold? O_o I thought the author's inlining it as it's a couple
locs/intstructions, while the compilers would most likely keep it
non-inlined as it's referenced 4 times. __cold will for sure keep it
standalone and place it in .text.cold, i.e. far away from the call sites.
I realize dev_core_stats_*() aren't called frequently, but why making
only one small helper cold rather than all of them then?

> 
>> +{
>> +       /* This READ_ONCE() pairs with the write in netdev_core_stats_alloc() */
>> +       struct net_device_core_stats __percpu *p = READ_ONCE(dev->core_stats);
>> +
>> +       if (likely(p))
>> +               return p;
>> +
>> +       return netdev_core_stats_alloc(dev);
>> +}

[...]

Thanks,
Olek