Date:   Tue, 14 Mar 2023 12:37:45 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     Jason Xing <kerneljasonxing@gmail.com>
Cc:     davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, ast@kernel.org, daniel@iogearbox.net,
        hawk@kernel.org, john.fastabend@gmail.com,
        stephen@networkplumber.org, sinquersw@gmail.com,
        bpf@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, Jason Xing <kernelxing@tencent.com>
Subject: Re: [PATCH v2 net-next 1/2] net-sysfs: display two backlog queue len
 separately
Message-ID: <ZBBciS/flAz2cPBq@corigine.com>
References: <20230314030532.9238-1-kerneljasonxing@gmail.com>
 <20230314030532.9238-2-kerneljasonxing@gmail.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230314030532.9238-2-kerneljasonxing@gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?iYlTeqfpRLobVHSjKJKIfvLUomfR/Q2lYXj3RZvCJk5uQjvNUC/U8AO+E3pM?=
 =?us-ascii?Q?dMAmXBib24UxOJvsAQysWHawWMCpvjrpCmDW6gZmhOB9Ur3bHz8rfRSitHJu?=
 =?us-ascii?Q?XT8zH41qrW7a1zCFePLtumjWbotn2cYeVMUhzz1ihHzXIrHtv0a1KUn+8cA0?=
 =?us-ascii?Q?OYtXWa/9KL9UoPAUxNKfsBmI36w7zLEG8zLBMYKnZ9Ma3eg1Z9jAN1USDvgv?=
 =?us-ascii?Q?iUbAnIcTMVwjpHKmRTIJZ4aQzO6XzRKtc6p+VwZe7EKsLFlVF0lkudS74bR4?=
 =?us-ascii?Q?RUC8rhAcIswyy0D2ig5LkoOPvkr29m+Vo7dAZwrQGTpByiCIDQdbscvVImmp?=
 =?us-ascii?Q?wZYkgyIRwywZSS3DjIxctsHcLMSpevDsh+RPm2bqQYZaQnN5jTk3rGtKWSag?=
 =?us-ascii?Q?ywT9PD/FnQZjqwxvkI6h30OkUc3Q+TfWf2+0pKHVMDou0/n0aYG3JSFKfbgY?=
 =?us-ascii?Q?D4I6Uw74edHySw01i/ektZZSthRH1PnOq7Hs5mSC3+FwW6a88v4QjnkoFH6i?=
 =?us-ascii?Q?9H7apj+6BY+VLbt+PfDKbkhviGyuCfuhRofnmF0w6DPnAuOd5d5gWaXzrudo?=
 =?us-ascii?Q?/fnoECXg+LPOu5tHA3sKLKI/PKPlHqAbXSEsqEXKRWkw2QTVHiQrDpCJrL/y?=
 =?us-ascii?Q?/LvBuZxu/tP6xKvZyR1OD00uckqribG7Op4RWlxCo5h4ytX4QlsGpCfO1nbT?=
 =?us-ascii?Q?dXmQv+4Hw/gW0oSHrrp2KF7+mSvBtqqtS5vYVdpsrJXlaRt4ZQ41KOjj9/kB?=
 =?us-ascii?Q?wcV5xRTljjSyDs2/mIQv5353925VzbJgu4jreVXDV6f86JfjR3yxsSYpnxDd?=
 =?us-ascii?Q?/VhiNOavtfioNYP6KKmTiyfDlsnX7uH8YkrFFQQsc+xBz+JVfUeIwoMSvdgR?=
 =?us-ascii?Q?3jUHW7afiXoPTnMKROH2iRopLbknMw+ol/0F1q3ObR85KDIUCj7FnMX2KnW7?=
 =?us-ascii?Q?gcI65SixeDP9Tj0EBKeYtg99XRmCeWjAgnjrwF+6pKEv81GAWt/Di7yA4xVY?=
 =?us-ascii?Q?v1JcvIM4l2eJowretWM8w8ElL75F/ryKoxW6KxsjEdZvLSW5KNZ8jMqo1N5R?=
 =?us-ascii?Q?EnENxmjmOMM4QuCMBNaQFGtQ+5uDX4bi6ydWetCeIDOLTJa9Z1wHbncjucEz?=
 =?us-ascii?Q?BnowD1jAXm6DPQwAcLEg3yvRITmQDFTQA0iCtlgAxgYVJ0RuSQdzLdgP4wVH?=
 =?us-ascii?Q?K7eW/jDhMpIZ74XJsr9uJ8YbOFOw8G0ZAoPGXYle1ZMLCMsvGegRBVQVW6mY?=
 =?us-ascii?Q?qRNUrmYLipUsOUWmDIF2/D4DN3Kw9zOkEVbxW6gEBIPbEhoIns2MZ3F/LeR/?=
 =?us-ascii?Q?1jGE4G862h049MOUTY1SNPP9MBzgwv2+RMZ7AMRaeMr5ObG07zaYJX/NObxl?=
 =?us-ascii?Q?+ErHoVc9cDF8tl5o27oeGTWFYBU664E0ePrDpCy0waOZ5wJYFf41rZzGXTB4?=
 =?us-ascii?Q?7/+o+jmCEpKsvoGWD/VewdVoVBxbUcTRnosjGwCVPM58ipRIgDzStZ82dkuY?=
 =?us-ascii?Q?irOoZe7KTEaytL0G+2zGoRNIou0zDMhc10056LvCW8PbCwXLNmIrXfoNyvVp?=
 =?us-ascii?Q?f1Mg9HiWYV06PvCu+CJxy2HjuzqcmOl8kWekw9t1inS8WcOD/0ywFPCkE2v5?=
 =?us-ascii?Q?fHFeBVsNEMI7M1kKU2ZwDAssP1keJ+444uX1xDs9t/bDgo4dDKgT3/MpdhmX?=
 =?us-ascii?Q?ZAz4zg=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 676a19b5-5c9b-45b7-2fd1-08db24808bd5
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Mar 2023 11:37:52.5206
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 4rdf1majWkrOzbB0ecWy/bV2bsarFOSP7UHNEAzZBbQr3FLD8N9So1maUVE2JpYg2nABWN0NfmQ7bD9Zwz5BWpcCAKJBB0jWd4XXgOraYkc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR13MB6148
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Mar 14, 2023 at 11:05:31AM +0800, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> Sometimes we need to know which one of backlog queue can be exactly
> long enough to cause some latency when debugging this part is needed.
> Thus, we can then separate the display of both.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> v2: keep the total len of backlog queues untouched as Eric said
> Link: https://lore.kernel.org/lkml/20230311151756.83302-1-kerneljasonxing@gmail.com/
> ---
>  net/core/net-procfs.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/net/core/net-procfs.c b/net/core/net-procfs.c
> index 1ec23bf8b05c..2809b663e78d 100644
> --- a/net/core/net-procfs.c
> +++ b/net/core/net-procfs.c
> @@ -115,10 +115,19 @@ static int dev_seq_show(struct seq_file *seq, void *v)
>  	return 0;
>  }
>  
> +static u32 softnet_input_pkt_queue_len(struct softnet_data *sd)
> +{
> +	return skb_queue_len_lockless(&sd->input_pkt_queue);
> +}
> +
> +static u32 softnet_process_queue_len(struct softnet_data *sd)
> +{
> +	return skb_queue_len_lockless(&sd->process_queue);
> +}
> +
>  static u32 softnet_backlog_len(struct softnet_data *sd)
>  {
> -	return skb_queue_len_lockless(&sd->input_pkt_queue) +
> -	       skb_queue_len_lockless(&sd->process_queue);
> +	return softnet_input_pkt_queue_len(sd) + softnet_process_queue_len(sd);
>  }
>  
>  static struct softnet_data *softnet_get_online(loff_t *pos)
> @@ -169,12 +178,15 @@ static int softnet_seq_show(struct seq_file *seq, void *v)
>  	 * mapping the data a specific CPU
>  	 */
>  	seq_printf(seq,
> -		   "%08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x\n",
> +		   "%08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x "
> +		   "%08x %08x\n",
>  		   sd->processed, sd->dropped, sd->time_squeeze, 0,
>  		   0, 0, 0, 0, /* was fastroute */
>  		   0,	/* was cpu_collision */
>  		   sd->received_rps, flow_limit_count,
> -		   softnet_backlog_len(sd), (int)seq->index);
> +		   softnet_backlog_len(sd),	/* keep it untouched */

I'm not sure the comment on the line above buys
us much outside of the context of development of this patch.

Likewise in patch 2/2.

That not withstanding, this looks good to me.

Reviewed-by: Simon Horman <simon.horman@corigine.com>


> +		   (int)seq->index,
> +		   softnet_input_pkt_queue_len(sd), softnet_process_queue_len(sd));
>  	return 0;
>  }
>  
> -- 
> 2.37.3
>