netdev - RE: [PATCH net-next 1/2] net: Keep sk->sk_forward

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CH3PR11MB734512D5836DBA1F1F3AE7CDFC759@CH3PR11MB7345.namprd11.prod.outlook.com>
Date: Fri, 12 May 2023 02:38:46 +0000
From: "Zhang, Cathy" <cathy.zhang@...el.com>
To: Shakeel Butt <shakeelb@...gle.com>, "Zhang@...gle.com" <Zhang@...gle.com>
CC: Eric Dumazet <edumazet@...gle.com>, Linux MM <linux-mm@...ck.org>, Cgroups
	<cgroups@...r.kernel.org>, Paolo Abeni <pabeni@...hat.com>,
	"davem@...emloft.net" <davem@...emloft.net>, "kuba@...nel.org"
	<kuba@...nel.org>, "Brandeburg@...gle.com" <Brandeburg@...gle.com>,
	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>, "Srinivas@...gle.com"
	<Srinivas@...gle.com>, "Srinivas, Suresh" <suresh.srinivas@...el.com>,
	"Chen@...gle.com" <Chen@...gle.com>, "Chen, Tim C" <tim.c.chen@...el.com>,
	"You@...gle.com" <You@...gle.com>, "You, Lizhen" <lizhen.you@...el.com>,
	"eric.dumazet@...il.com" <eric.dumazet@...il.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>
Subject: RE: [PATCH net-next 1/2] net: Keep sk->sk_forward_alloc as a proper
 size



> -----Original Message-----
> From: Shakeel Butt <shakeelb@...gle.com>
> Sent: Friday, May 12, 2023 5:19 AM
> To: Zhang@...gle.com; Zhang, Cathy <cathy.zhang@...el.com>
> Cc: Eric Dumazet <edumazet@...gle.com>; Linux MM <linux-
> mm@...ck.org>; Cgroups <cgroups@...r.kernel.org>; Paolo Abeni
> <pabeni@...hat.com>; davem@...emloft.net; kuba@...nel.org;
> Brandeburg@...gle.com; Brandeburg, Jesse <jesse.brandeburg@...el.com>;
> Srinivas@...gle.com; Srinivas, Suresh <suresh.srinivas@...el.com>;
> Chen@...gle.com; Chen, Tim C <tim.c.chen@...el.com>; You@...gle.com;
> You, Lizhen <lizhen.you@...el.com>; eric.dumazet@...il.com;
> netdev@...r.kernel.org
> Subject: Re: [PATCH net-next 1/2] net: Keep sk->sk_forward_alloc as a proper
> size
> 
> On Thu, May 11, 2023 at 09:26:46AM +0000, Zhang, Cathy wrote:
> >
> [...]
> >
> >      8.98%  mc-worker        [kernel.vmlinux]          [k] page_counter_cancel
> >             |
> >              --8.97%--page_counter_cancel
> >                        |
> >                         --8.97%--page_counter_uncharge
> >                                   drain_stock
> >                                   __refill_stock
> >                                   refill_stock
> >                                   |
> >                                    --8.91%--try_charge_memcg
> >                                              mem_cgroup_charge_skmem
> 
> I do want to understand for above which specific condition in __refill_stock is
> causing to drain stock in the charge code path. Can you please re-run and
> profile your test with following code snippet (or use any other mechanism
> which can answer the question)?
> 
> From f1d91043f21f4b29717c78615b374d79fc021d1f Mon Sep 17 00:00:00
> 2001
> From: Shakeel Butt <shakeelb@...gle.com>
> Date: Thu, 11 May 2023 20:00:19 +0000
> Subject: [PATCH] Debug drain on charging.
> 
> ---
>  mm/memcontrol.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c index
> d31fb1e2cb33..4c1c3d90a4a3 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2311,6 +2311,16 @@ static void drain_local_stock(struct work_struct
> *dummy)
>  		obj_cgroup_put(old);
>  }
> 
> +static noinline void drain_stock_1(struct memcg_stock_pcp *stock) {
> +	drain_stock(stock);
> +}
> +
> +static noinline void drain_stock_2(struct memcg_stock_pcp *stock) {
> +	drain_stock(stock);
> +}
> +
>  /*
>   * Cache charges(val) to local per_cpu area.
>   * This will be consumed by consume_stock() function, later.
> @@ -2321,14 +2331,14 @@ static void __refill_stock(struct mem_cgroup
> *memcg, unsigned int nr_pages)
> 
>  	stock = this_cpu_ptr(&memcg_stock);
>  	if (READ_ONCE(stock->cached) != memcg) { /* reset if necessary */
> -		drain_stock(stock);
> +		drain_stock_1(stock);
>  		css_get(&memcg->css);
>  		WRITE_ONCE(stock->cached, memcg);
>  	}
>  	stock->nr_pages += nr_pages;
> 
>  	if (stock->nr_pages > MEMCG_CHARGE_BATCH)
> -		drain_stock(stock);
> +		drain_stock_2(stock);
>  }
> 
>  static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
> --
> 2.40.1.606.ga4b1b128d6-goog

Hi Shakeel,

Run with the temp change you provided,  the output shows it comes to drain_stock_1(),
Here is the call trace:

     8.96%  mc-worker        [kernel.vmlinux]            [k] page_counter_cancel
            |
             --8.95%--page_counter_cancel
                       |
                        --8.95%--page_counter_uncharge
                                  drain_stock_1
                                  __refill_stock
                                  refill_stock
                                  |
                                   --8.88%--try_charge_memcg
                                             mem_cgroup_charge_skmem
                                             |
                                              --8.87%--__sk_mem_raise_allocated
                                                        __sk_mem_schedule
                                                        |
                                                        |--5.37%--tcp_try_rmem_schedule
                                                        |          tcp_data_queue
                                                        |          tcp_rcv_established
                                                        |          tcp_v4_do_rcv