Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 21 Apr 2021 16:55:34 -0700
From:   Roman Gushchin <guro@fb.com>
To:     Waiman Long <longman@redhat.com>
CC:     Johannes Weiner <hannes@cmpxchg.org>,
        Michal Hocko <mhocko@kernel.org>,
        Vladimir Davydov <vdavydov.dev@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Tejun Heo <tj@kernel.org>, Christoph Lameter <cl@linux.com>,
        Pekka Enberg <penberg@kernel.org>,
        David Rientjes <rientjes@google.com>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        <linux-kernel@vger.kernel.org>, <cgroups@vger.kernel.org>,
        <linux-mm@kvack.org>, Shakeel Butt <shakeelb@google.com>,
        Muchun Song <songmuchun@bytedance.com>,
        Alex Shi <alex.shi@linux.alibaba.com>,
        Chris Down <chris@chrisdown.name>,
        Yafang Shao <laoar.shao@gmail.com>,
        Wei Yang <richard.weiyang@gmail.com>,
        Masayoshi Mizuma <msys.mizuma@gmail.com>,
        Xing Zhengjun <zhengjun.xing@linux.intel.com>,
        Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH-next v5 3/4] mm/memcg: Improve refill_obj_stock()
 performance
Message-ID: <YIC7dh0+nQDFmU+T@carbon.dhcp.thefacebook.com>
References: <20210420192907.30880-1-longman@redhat.com>
 <20210420192907.30880-4-longman@redhat.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20210420192907.30880-4-longman@redhat.com>
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from carbon.dhcp.thefacebook.com (2620:10d:c090:400::5:4579) by MW2PR16CA0002.namprd16.prod.outlook.com (2603:10b6:907::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4065.20 via Frontend Transport; Wed, 21 Apr 2021 23:55:38 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 78d83adb-835f-4316-a044-08d90520f787
X-MS-TrafficTypeDiagnostic: SJ0PR15MB4469:
X-Microsoft-Antispam-PRVS: <SJ0PR15MB4469C09B11CDAD6C3844A12ABE479@SJ0PR15MB4469.namprd15.prod.outlook.com>
X-FB-Source: Internal
X-MS-Oob-TLC-OOBClassifiers: OLM:7691;
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: UPWg/HNj3Mv7PzqSfYIp4kWomF0/fmsfY64FG6hN3Jj/OyGDC/q6VE33ypCcNWauzrfY+zMwJBl6d8zbYiZpBHbcQ1PoQjQXC84s1dkbYLQDLeLzyh9Txy3d84SFfDHLMNk3i0LS+99lEIy/ykxngrgqN15+5/WRdePUALKpIL7GuFDc2dtZ/XLrnXczM4T5H2O16nXNKjDlLvl2q9LHXsOe+7DZJtxI8KsZcsysxkVcvWxHKbc6NjcvGoHqofncH/hDho1QV7oUayXI56yDhIUhqjf4z3E8iC6ShLof2b6nfzsVFT4br06CKk2PJs263iNd3E55sCt/jojS8qLSA2KMNw1cOQE3vHtDsdkve5c7qiSfwjFoqpm3S+Y863oj1u4bHTqNgq+h1b3hKWNPZ6hW1/QN5y2vdRj3eOwObPKvOK5665SfHnD4KzKZRsiPEJzKeSC3JFNhvobgeZ5AO8qtQwxEOKHBP2wMg1J4EazyCXxc9mNFoZdWRaTce3TsnZdL4w07H4yDj9G/2EuR+aSgj7IvpE13E+or3UDZClW1Bw1uMTzsyB6PZ+nnU20ABbTIqURePYg2+qkl0xwx+pVeGJV9M2VNO3rDSCJgCvc=
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BYAPR15MB4136.namprd15.prod.outlook.com;PTR:;CAT:NONE;SFS:(366004)(396003)(346002)(136003)(376002)(39860400002)(6666004)(66946007)(54906003)(7696005)(186003)(8936002)(86362001)(52116002)(6506007)(478600001)(38100700002)(16526019)(7416002)(9686003)(66476007)(66556008)(316002)(4326008)(5660300002)(8676002)(6916009)(2906002)(55016002);DIR:OUT;SFP:1102;
X-MS-Exchange-AntiSpam-MessageData: =?us-ascii?Q?RHjyQZOu4reTBOIX336wtORVINdxnjtLw3Q4gEJyhrD87y651fkiFyJHVkN+?=
 =?us-ascii?Q?X2pjdpd5Q3L+ykMrHx6ybhXxLw+TGEUOQlUFkfEDZE4ZIZcJVK00F54bwZm6?=
 =?us-ascii?Q?PoOiU+IoLcPkcFueXvwKWqhLLv5MKs1CRgrC6pA/ohdYfPxTXmjs9ID0Mjh0?=
 =?us-ascii?Q?YZKZxNpd9PNa9ZzreJ5rpl3HF2wk0H1pgwzCaQ3lYyCSYHZ1bejZMV5WuLEJ?=
 =?us-ascii?Q?97pdMm2OwgqGxvPvxu0e6GtGHSnsKQpYgVRx2ndfgmDyMeW9GhKhh1/pM478?=
 =?us-ascii?Q?FNF8Xa/yFHhZtq1hjR3YuGGNW40ZH0lPGebmItvFwERhpkYHCmOGgPIThzC/?=
 =?us-ascii?Q?1SACh13yjDROgwP7SHPOFkF0e53UWyb/w0xpArQxv+S2zFqI2wInwXnIeCdy?=
 =?us-ascii?Q?BvvpyitY5pnbJnULf+5hgat5zvEBQOwFDyKbDdt9rw/VblEg7EpxGZZyQI30?=
 =?us-ascii?Q?2Vvbevxe/3uvriwNEGZJ+p58fFXjqN6od6gCgqCSaCuF09uO5wIzHj9PUk9d?=
 =?us-ascii?Q?rB3mOkDXYDnLM4neM7NBzUd1DCSvMEm5pgkwE4Xm7lK6685Xmn1CJdY14IGp?=
 =?us-ascii?Q?+kGnFyUPgATsXMPjZVgyID8Ct6SQYutKrr3Ng7gOu75LedEEfZTt6QBpyNuO?=
 =?us-ascii?Q?JEDs5DxydjmOi9ypZZSnNLBEi67OR8CrMaCnti30mwcp/JQHBdkNaXGYX9RV?=
 =?us-ascii?Q?nDIvvaQMslPA38sppngI0Ft2ny5wLz2P6t/+mSJ2Hi4/OSSkicVYC5UF09YK?=
 =?us-ascii?Q?rKi9j2CMZxhwHXgX5kezu7HLBDs6hFq8S6YMOwf2Sepi9hdmMrzXMyxp/R2I?=
 =?us-ascii?Q?VdC4vsJy/sZbDasb1PrShUKzLTf4jfFbhi9Hf8GsMCcgtarbevJTq6o6dkL+?=
 =?us-ascii?Q?DfgybkLrYl5scs+sEBG4HT4IGTG5uM2c7RVMQ+dgglmKy5AKIJ3jcCwNqi/A?=
 =?us-ascii?Q?lmQb/U8Gj5zSA8ZDpgLPfhePnNWS/4svX1uXv1CScAh/3LbNXXgRfgc3mvV3?=
 =?us-ascii?Q?m54v1crvWNN+swojitoGw0ppBc9HJtNsvmDS4IKfqMhQgRYEZz1uG5jn1EN/?=
 =?us-ascii?Q?luyUR6mmSHlOCWnd40UJUZU0jsKu8tmJ7dNa4LWVVDndxQlC2M6NxwKqGxnE?=
 =?us-ascii?Q?xzeR0H/xkxUWd2awYesyxwFZ4sgKGxxA7KG/qIEzkm/ajr7KxNfxBKVfSevy?=
 =?us-ascii?Q?+NnVbKV4PBLvFBdy/6oWUKsO/SCf1AEZRlWkjhGtj0MndESfqURxxUn8IRR9?=
 =?us-ascii?Q?P/gwOOm8bzLMmWTkz0u6KD27HuJXJ/EITDfS/sUOJFTZFU0pj9tP15S5lbse?=
 =?us-ascii?Q?TagLhUNwMndqiAZljFBOWx801uVQ0ehHiDvnmf/0EKwy9A=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 78d83adb-835f-4316-a044-08d90520f787
X-MS-Exchange-CrossTenant-AuthSource: BYAPR15MB4136.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Apr 2021 23:55:40.2087
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: MD1Fx7aRDH7VWYfcxnvppYt+0y6UzPKOlHbRwfQ64WhfnSqYzEO/2OTUiBmbPl2i
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR15MB4469
X-OriginatorOrg: fb.com
X-Proofpoint-GUID: FpGWz8gN1kx6WbjLfCKlKl2Hz129IfEI
X-Proofpoint-ORIG-GUID: FpGWz8gN1kx6WbjLfCKlKl2Hz129IfEI
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.761
 definitions=2021-04-21_08:2021-04-21,2021-04-21 signatures=0
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 impostorscore=0
 adultscore=0 mlxscore=0 spamscore=0 clxscore=1015 priorityscore=1501
 mlxlogscore=893 malwarescore=0 bulkscore=0 suspectscore=0 phishscore=0
 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2104060000 definitions=main-2104210158
X-FB-Internal: deliver
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Apr 20, 2021 at 03:29:06PM -0400, Waiman Long wrote:
> There are two issues with the current refill_obj_stock() code. First of
> all, when nr_bytes reaches over PAGE_SIZE, it calls drain_obj_stock() to
> atomically flush out remaining bytes to obj_cgroup, clear cached_objcg
> and do a obj_cgroup_put(). It is likely that the same obj_cgroup will
> be used again which leads to another call to drain_obj_stock() and
> obj_cgroup_get() as well as atomically retrieve the available byte from
> obj_cgroup. That is costly. Instead, we should just uncharge the excess
> pages, reduce the stock bytes and be done with it. The drain_obj_stock()
> function should only be called when obj_cgroup changes.

I really like this idea! Thanks!

However, I wonder if it can implemented simpler by splitting drain_obj_stock()
into two functions:
     empty_obj_stock() will flush cached bytes, but not reset the objcg
     drain_obj_stock() will call empty_obj_stock() and then reset objcg

Then we simple can replace the second drain_obj_stock() in
refill_obj_stock() with empty_obj_stock(). What do you think?

> 
> Secondly, when charging an object of size not less than a page in
> obj_cgroup_charge(), it is possible that the remaining bytes to be
> refilled to the stock will overflow a page and cause refill_obj_stock()
> to uncharge 1 page. To avoid the additional uncharge in this case,
> a new overfill flag is added to refill_obj_stock() which will be set
> when called from obj_cgroup_charge().
> 
> A multithreaded kmalloc+kfree microbenchmark on a 2-socket 48-core
> 96-thread x86-64 system with 96 testing threads were run.  Before this
> patch, the total number of kilo kmalloc+kfree operations done for a 4k
> large object by all the testing threads per second were 4,304 kops/s
> (cgroup v1) and 8,478 kops/s (cgroup v2). After applying this patch, the
> number were 4,731 (cgroup v1) and 418,142 (cgroup v2) respectively. This
> represents a performance improvement of 1.10X (cgroup v1) and 49.3X
> (cgroup v2).

This part looks more controversial. Basically if there are N consequent
allocations of size (PAGE_SIZE + x), the stock will end up with (N * x)
cached bytes, right? It's not the end of the world, but do we really
need it given that uncharging a page is also cached?

Thanks!