| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 15 Jan 2024 10:22:03 +0800
From: "Yin, Fengwei" <fengwei.yin@...el.com>
To: 王珂琦 Keqi Wang Wang
<wangkeqiwang@...iglobal.com>, kernel test robot <oliver.sang@...el.com>
CC: "oe-lkp@...ts.linux.dev" <oe-lkp@...ts.linux.dev>, "lkp@...el.com"
<lkp@...el.com>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "ying.huang@...el.com"
<ying.huang@...el.com>, "feng.tang@...el.com" <feng.tang@...el.com>
Subject: Re: [linus:master] [connector] c46bfba133:
stress-ng.netlink-proc.ops_per_sec -97.2% regression
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@...iglobal.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@...iglobal.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@...el.com <mailto:fengwei.yin@...el.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
>
>
>
Powered by blists - more mailing lists