Received-SPF: pass (google.com: domain of linux-kernel+bounces-25590-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <388ff39f-c4d5-4d71-8c5e-2a3d242d8bf1@intel.com>
Date: Mon, 15 Jan 2024 10:22:03 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [linus:master] [connector] c46bfba133:
 stress-ng.netlink-proc.ops_per_sec -97.2% regression
To: =?UTF-8?B?546L54+C55CmIEtlcWkgV2FuZyBXYW5n?=
	<wangkeqiwang@didiglobal.com>, kernel test robot <oliver.sang@intel.com>
CC: "oe-lkp@lists.linux.dev" <oe-lkp@lists.linux.dev>, "lkp@intel.com"
	<lkp@intel.com>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "David S. Miller" <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>, "ying.huang@intel.com"
	<ying.huang@intel.com>, "feng.tang@intel.com" <feng.tang@intel.com>
References: <E0375D03-BC1E-4F78-8D44-83F4E98307BC@didiglobal.com>
Content-Language: en-US
From: "Yin, Fengwei" <fengwei.yin@intel.com>
In-Reply-To: <E0375D03-BC1E-4F78-8D44-83F4E98307BC@didiglobal.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?akl4LzFyNk5KZmVVSTR4cWdyN1NkdnhsUlhLdUNkaUZncmxaTStoSFk3MVVI?=
 =?utf-8?B?MzZLZlVPNDhTQWRoYjkwNktUVXBTcUdURDlFYVJGNXNKVExZejVIU083YmdT?=
 =?utf-8?B?dXRHVi9lVStkRGs0K1Z4Sm1DWi9iYkRyT3YzQnYrYmlCYnpBclhWcjdWazQx?=
 =?utf-8?B?T3lXUmEzbXlKQmJicXlZeVJwd2ZmdEdXeDhOdFFucUxjamNRMW5ZaVI1bjVn?=
 =?utf-8?B?eUplbGRqbTl5VHUvc2tLRktnUjVzcThtUHRPYkdVdGtUcE1OZ0JYSDdiVGN4?=
 =?utf-8?B?aGdKQkZWdGlhSkt2UjlVTTNJZ0FSQzFTY2hlQTgrR0NhTjJIZHMzc3d1Nlh3?=
 =?utf-8?B?OEJuSU8yVHgyNE9iQUZzaHVwcXVpMjhoRlk0amNWNFZKaVZaSFF2Yk5GVXJF?=
 =?utf-8?B?NmlydTJIN1VkYVV4dG9aaVgxWkNOemhOLzE3T3NWU0hNR29ETUhheXN1UlNJ?=
 =?utf-8?B?MitMNW5wbnZGT0gzNmUxWEhrazJCWkgzZEhFMnNCM2dTYVArZXZjbXhNbm83?=
 =?utf-8?B?UUd6M2tOZ0d0TUdBMEREcXM3Y1RoQXV6YWc1NEtFNTJqMm9EcVUxSDc5ME5J?=
 =?utf-8?B?TGFKZW00a2dmNTZKelVWalA2WDVTc0F6VHZ6WThCdEprREV3T2lnNUpCTzUy?=
 =?utf-8?B?azBkNExTNWpaTURHWkFqMHpYN2Z0bXIyV1hLYVhWWkhEVHVsVGJQSlJ0T0hr?=
 =?utf-8?B?aVo5TTFQWkZ5SHJsWE95YWxlSXJlRWd1L1RIRTZ1UU14bzhYRG9RaUdoVTla?=
 =?utf-8?B?eEYvY2x6aWFXVE1qcXRkR1ZuY1FyeTBmMXJuUDI2RHhPaWlHSTFoSHJXQWRa?=
 =?utf-8?B?U2QvOXN1SFR4c1Nmd1p1TlpuSSttekJmRXNPK1ZnQTJZMERPZlFWT25YMDNR?=
 =?utf-8?B?THpoOEhFQjNCMHlLTUE1ZmcrVHVnZUpseXdDaGc5K1VqeGhsekxMcHBXT1N5?=
 =?utf-8?B?T1NQZWRjdHVOblBnc2Vyd3FIRGFGSXM1YXVqQkpSVnljTlphTEh4OUViVTVT?=
 =?utf-8?B?SmdVbGt1VzBaK004Wi9zeVpqWXlXT05TLzNQT1hBQmNGZ3gvdUVNdFFtSkNp?=
 =?utf-8?B?YUZKeWh2WnRRWmtSZXhaVEN1d1Z0clh2VDZDNjVFY0ExNXlXTEtnblhudFJ4?=
 =?utf-8?B?SktzQ0txK09lQ0lXa2JvcVJXWEZneGJkd2J1a1dTQ3QwY0ZySTExZFNEMGpX?=
 =?utf-8?B?aXMrSWJNeDVpMmpGYVZSZVA3RlI4dXJWMXhjMHBZa2pRL2lFLzQ2bTVXanFR?=
 =?utf-8?B?ZDdkYThuRzhLM2x0MUpuY3E5TTVMVEg1RSszZ0tRZTM3SE1NaWdTL1ZHRlI3?=
 =?utf-8?B?S3FySSswMXk4akd0ZWVwQ3I0L3I4VGxpUHBuTHd0T0FYVVNtQmNNdDBFUDJV?=
 =?utf-8?B?cWdLNlNKZXpEbVNIRmNvQm1FR2dxaFNJR3dmbFU1SWpxL2MveWdodnRJL2h1?=
 =?utf-8?B?QlBLbng1MzZsczUxdWRvRnhXa215R25UNEt0emxOMHJRZGYwTnRFaVFQRktQ?=
 =?utf-8?B?d1oraCtrcWVJaHlCZzlEZkExVmkvRHQ1Vlg5bWRyNEJjWVVFazdsVWR6NVVl?=
 =?utf-8?B?N0RNcjdZTnF5Zm5wMC9CcTFVSTQ4SFlmZXBaN0RNTmpjV1crbVdZbG1uYkFN?=
 =?utf-8?B?ZWYxSzlaelZDdW5wQTQyUjZ1bzF0Qjc3VU1TOXlnT3NUVTVFb0hEQ3dWUjgx?=
 =?utf-8?B?NitVVS9DSlNkSFlIZHFsZXorbzc4TnRubjJ6VHQyREh0cEdmTDkxZTJIS0c4?=
 =?utf-8?B?bVNjaWxJNSttZW5MWkdPeU9sVXVZaGlobkZNS3VJS0VwOHJrZWZEbitITktK?=
 =?utf-8?B?NlF0L1lTeFdaak1zbWJoQURKSHdVVGZ4Vnpsb2F0ckIrQVFIa083OVBCc1V6?=
 =?utf-8?B?S0RMVnR0V3JZRFJFREJNNmFuTUl2d2RxRzlwekpFcVQydFFWN2N5T1JQL3l4?=
 =?utf-8?B?OTlvYjR3YUM5elpmTnEvdjlEcy9KOEZCM3VLUzBta214TmtyanZsWTYrL0ZQ?=
 =?utf-8?B?Q2VMcCsrN0V6MVg0bVJVN1p4RmxMc21mZk5sb1hqVCtqNVhyWlZRSllWcUh1?=
 =?utf-8?B?VHhqcnJiejBWMmhRTWxpV3Vyc2x2aTVlYzRzUDYyL0FpekQxUDhSV1FHWDlo?=
 =?utf-8?B?SHdCQzJxUDJQZDYwWnRmTHU0ek5vWldrQ0Z6cjNad29mcjMvc0dqcjRESVhH?=
 =?utf-8?B?SkE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: bd0edc8a-6852-4f9b-e31a-08dc1570c77d
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4820.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jan 2024 02:22:11.0356
 (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: XkGEMrIfgWFrb1IjHblhTu/T1C7cvO1BsEYTf5K2EeXQRi92+EE42i5xXHTdbTxrtTbLoJ0pBb6B3Mpm+y5emQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB4928
X-OriginatorOrg: intel.com


On 1/13/2024 4:56 PM, 王珂琦 Keqi Wang Wang wrote:
> Hi Fengwei
> 	Sorry, reply so late.
No problem.

> 	I don't think this will cause a drastic drop in stress-ng netlink-proc performance. Because after returning -ESRCH, proc_event_num_listeners is cleared and the send_msg function will not be called.
> However, there is a problem with judging clearing based on whether the return value is -ESRCH. Because netlink_broadcast will return -ESRCH in this case. Can you try the following patch to solve your problem?
Yes. This patch can make the regression of stress-ng.netlink-proc gone.


Regards
Yin, Fengwei

> 
>  From 6e6c36aed156bbb185f54d0c0fef2f6683df3288 Mon Sep 17 00:00:00 2001
> From: wangkeqi <wangkeqiwang@didiglobal.com>
> Date: Sat, 23 Dec 2023 13:21:17 +0800
> Subject: [PATCH] connector: Fix proc_event_num_listeners count not cleared
> 
> When we register a cn_proc listening event, the proc_event_num_listener
> variable will be incremented by one, but if PROC_CN_MCAST_IGNORE is
> not called, the count will not decrease.
> This will cause the proc_*_connector function to take the wrong path.
> It will reappear when the forkstat tool exits via ctrl + c.
> We solve this problem by determining whether
> there are still listeners to clear proc_event_num_listener.
> 
> Signed-off-by: wangkeqi <wangkeqiwang@didiglobal.com>
> ---
>   drivers/connector/cn_proc.c   | 5 ++++-
>   drivers/connector/connector.c | 6 ++++++
>   include/linux/connector.h     | 1 +
>   3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
> index 44b19e696..b09f74ed3 100644
> --- a/drivers/connector/cn_proc.c
> +++ b/drivers/connector/cn_proc.c
> @@ -108,8 +108,11 @@ static inline void send_msg(struct cn_msg *msg)
>   		filter_data[1] = 0;
>   	}
> 
> -	cn_netlink_send_mult(msg, msg->len, 0, CN_IDX_PROC, GFP_NOWAIT,
> +	if (netlink_has_listeners(get_cdev_nls(), CN_IDX_PROC))
> +		cn_netlink_send_mult(msg, msg->len, 0, CN_IDX_PROC, GFP_NOWAIT,
>   			     cn_filter, (void *)filter_data);
> +	else
> +		atomic_set(&proc_event_num_listeners, 0);
> 
>   	local_unlock(&local_event.lock);
>   }
> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
> index 7f7b94f61..ced2655c6 100644
> --- a/drivers/connector/connector.c
> +++ b/drivers/connector/connector.c
> @@ -120,6 +120,12 @@ int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid, u32 __group,
>   }
>   EXPORT_SYMBOL_GPL(cn_netlink_send_mult);
> 
> +struct sock *get_cdev_nls(void)
> +{
> +	return cdev.nls;
> +}
> +EXPORT_SYMBOL_GPL(get_cdev_nls);
> +
>   /* same as cn_netlink_send_mult except msg->len is used for len */
>   int cn_netlink_send(struct cn_msg *msg, u32 portid, u32 __group,
>   	gfp_t gfp_mask)
> diff --git a/include/linux/connector.h b/include/linux/connector.h
> index cec2d99ae..c601eb99b 100644
> --- a/include/linux/connector.h
> +++ b/include/linux/connector.h
> @@ -126,6 +126,7 @@ int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid,
>    * If there are no listeners for given group %-ESRCH can be returned.
>    */
>   int cn_netlink_send(struct cn_msg *msg, u32 portid, u32 group, gfp_t gfp_mask);
> +struct sock *get_cdev_nls(void);
> 
>   int cn_queue_add_callback(struct cn_queue_dev *dev, const char *name,
>   			  const struct cb_id *id,
> --
> 2.27.0
> 
> ﻿在 2024/1/12 19:28，“Yin, Fengwei”<fengwei.yin@intel.com <mailto:fengwei.yin@intel.com>> 写入:
> 
> 
> 
> 
> 
> 
> On 1/11/2024 11:19 PM, kernel test robot wrote:
>>
>>
>> Hello,
>>
>> we reviewed this report and Fengwei (Cced) pointed out it could be the patch
>> breaks functionality, then causes stress-ng netlink-proc performance drops
>> dramatically.
>>
> Just FYI. Here is what I observed when running
> stress-ng.netlink-proc testing:
> 
> 
> Whatever with/without the patch, cn_netlink_send_mult() returns
> -ESRCH in most case.
> 
> 
> The following is what the cn_netlink_send_mult() returns when
> stress-ng.netlink-proc is running:
> 
> 
> ...
> 213801 213801 stress-ng-3 cn_netlink_send_mult -3
> 213801 213801 stress-ng-spawn cn_netlink_send_mult -3
> 213801 213801 stress-ng-spawn cn_netlink_send_mult -3
> 213801 213801 stress-ng-wait cn_netlink_send_mult -3
> 213802 213802 stress-ng-4 cn_netlink_send_mult -3
> 213802 213802 stress-ng-spawn cn_netlink_send_mult -3
> 213802 213802 stress-ng-spawn cn_netlink_send_mult -3
> 213802 213802 stress-ng-wait cn_netlink_send_mult -3
> 213803 213803 stress-ng-5 cn_netlink_send_mult -3
> 213803 213803 stress-ng-dead cn_netlink_send_mult -3
> 213803 213803 stress-ng-dead cn_netlink_send_mult -3
> 213802 213802 stress-ng-wait cn_netlink_send_mult -3
> 213801 213801 stress-ng-wait cn_netlink_send_mult -3
> 213800 213800 stress-ng-wait cn_netlink_send_mult -3
> 213799 213799 stress-ng-wait cn_netlink_send_mult -3
> 213798 213798 stress-ng-wait cn_netlink_send_mult -3
> 154697 154697 stress-ng cn_netlink_send_mult -3
> ...
> 
> 
> 
> 
> Looks like it's not accurate to reset proc_event_num_listeners
> according to cn_netlink_send_mult() return value -3.
> 
> 
> 
> 
> Regards
> Yin, Fengwei
> 
> 
>