From: "Yin, Fengwei" <fengwei.yin@intel.com>
To: "王珂琦 Keqi Wang Wang" <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>
Subject: Re: [linus:master] [connector] c46bfba133: stress-ng.netlink-proc.ops_per_sec -97.2% regression
Date: Mon, 15 Jan 2024 10:22:03 +0800 [thread overview]
Message-ID: <388ff39f-c4d5-4d71-8c5e-2a3d242d8bf1@intel.com> (raw)
In-Reply-To: <E0375D03-BC1E-4F78-8D44-83F4E98307BC@didiglobal.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
>
>
>
prev parent reply other threads:[~2024-01-15 2:22 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-11 15:19 [linus:master] [connector] c46bfba133: stress-ng.netlink-proc.ops_per_sec -97.2% regression kernel test robot
2024-01-12 11:28 ` Yin, Fengwei
2024-01-13 8:56 ` 王珂琦 Keqi Wang Wang
2024-01-15 2:22 ` Yin, Fengwei [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=388ff39f-c4d5-4d71-8c5e-2a3d242d8bf1@intel.com \
--to=fengwei.yin@intel.com \
--cc=davem@davemloft.net \
--cc=feng.tang@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp@intel.com \
--cc=netdev@vger.kernel.org \
--cc=oe-lkp@lists.linux.dev \
--cc=oliver.sang@intel.com \
--cc=wangkeqiwang@didiglobal.com \
--cc=ying.huang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).